Re: [Opensync-devel] [PATCH] Important fix to libgcal-0.9.5

[Chris F. <cd...@fo...>]

On Mon, Aug 30, 2010 at 01:22:01PM -0400, Adenilson Cavalcanti wrote:
> > The event type seemed to me to be an opaque type. ?i.e. I'm not supposed
> > to access the internals directly, right? ?So if all I have is the
> > edit_url, how should I delete with gcal_erase_event(gcal_t, gcal_event_t)?
> > Perhaps I'm supposed to call set_url() first?
> 
> Exactly, you should set the edit url in the gcal_event_t 'object'.

Thanks.  That should work.


> The changed method is something that I can't accept, because it will
> break with ABI. I used the concept of 'objects' in libgcal to hide
> implementation details from users and ensure that all memory allocated
> by the library would be freed by it.

I'm going to get slightly argumentative here, but I mean no harm.
And I don't mind if you don't apply the patch either, even for a trvial
reason that you just don't like it. :-)

But I don't believe that gcal_delete_event_url() will break the ABI.  (I think
"ABI" might be a C++ concept anyway, which doesn't affect libgcal.)
The patch only adds a function, does not remove any, nor does it change
any existing function prototypes.

I don't see how gcal_delete_event_url() would in any way cause leaked memory,
either.


As a side note, before you release a nice shiny version 1.0 of libgcal,
somebody should fix all the const-correctness issues in the API. :-)
It's already bit me in the opensync plugin once.  I can work up a patch
if you like.

Thanks for the feedback!
- Chris