[Freeassociation-devel] icalproperty_new_from_string and REQUEST-STATUS

[crass at berlios.de]

There is an issue with creating a REQUEST-STATUS property from a
string, which creates a icalrequeststatus value with an invalid debug
member.  I'm proposing that we change the way icalrequeststatus structs
should be used.  Specifically, the debug member should be a pointer to
a string owned by the icalrequeststatus struct and allocated on the
heap (freeable).  This change is potentially problematic because it
possible some code dealing with REQUEST-STATUS property values may need
to be changed (if not they will leak memory).  However, if this is not
changed, its likely they will access deallocated memory (if they try to
access the string pointed to by the debug member.

Here's the problem.  Let's say you have the following code:

p = icalproperty_new_from_string("REQUEST-STATUS:2.1;Success but
fallback taken  on one or more property  values.;booga");

This will copy the string argument into an allocated buffer wrapped in
component text and parse that.  The following call will eventually be
made:

icalreqstattype_from_string(<allocated string of REQUEST-STATUS>)

This will return a struct icalrequeststatus with a debug member
pointing into the allocated string.  This is all fine and well and the
property is created as you would expect, except that at the end of
icalproperty_new_from_string the temporarily allocated string to hold
the component wrapped request status is then deallocated (as it should
be).  This automatically makes the debug member's pointer invalid.  The
problem is the caller doesn't expect this and has no way of nothing
that its not valid.

These proposed changes have icalreqstattype_from_string create an
allocated buffer for the debug string, so that it does not point into
the passed string.  The debug buffer will be automatically freed when
its in a property that is freed via icalproperty_free.  However, it
will need to be manually freed when deallocating the icalrequeststatus
struct manually.

I'm not completely satisfied with the changes, in that there's still
too much special knowledge needed on the part of the library user.  But
I think its better than the current form, which was found via odd
behaviors in the regression tests.  Does anyone have a better
alternative solution?  Suggestions?

Glenn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: make_icalreqstattype_debug_allocated_string.patch
Type: text/x-patch
Size: 3290 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20100331/19af7ce3/attachment.bin>