Re: [openchange]Revision 1762 slightly unusable
Milan Crha <mcrha <at> redhat.com>
2010-05-10 07:55:48 GMT
On Sat, 2010-05-08 at 14:35 +1000, Brad Hards wrote:
> On Saturday 08 May 2010 06:08:28 am Milan Crha wrote:
> > Hi there,
> > I'm still in process of testing the PT_DOUBLE bug, with not much
> > success, but that might be Evolution's issue, I'll investigate more.
> Tell me more about what you're seeing. Is there a bugzilla entry somewhere?
> Can you get a network capture?
I'll tell you when I will know more, I'm facing some issues with recent
evolution changes preventing me from testing this properly.
> I'm undecided on this bit:
> + case PT_NULL:
> + case PT_OBJECT:
> + case PT_MV_SHORT:
> + /* types, which cannot be stored in mapi_SPropValue */
> + printf ("cast_mapi_SPropValue: Cannot cast prop value 0x%x to mapi
> type\n", sprop->ulPropTag & 0xFFFF);
> + break;
> Can you explain what you're trying to do here?
>From the code reading I understood that struct mapi_SPropValue and
struct SPropValue are not 1:1, some values which can be stored in struct
SPropValue are not possible to store in struct mapi_SPropValue, and/or
vice versa. I found these three as the beginning. It's preventing crash
for developer compiles.
> [In a previous email you basically asked "why not just do them all"? I don't
> mind doing it, I just don't like code I don't have any tests for. I just don't
> trust untested code, and don't want to commit it. That is why I'm relying on
> your testing for this.]
Yup, I understand that, I also do not like untested code.
One more thing I forgot to mention in my previous email: I noticed that
cast_mapi_SPropValue allocates memory on the global memory context, but
only partially, the array itself, but not array items, which might mean,
if I read and understand the core properly, that values in mapi_sprop
are valid only until the sprop value isn't freed. It is good and bad in
the same time. It's good as you do not allocate some memory twice (only
the array, with actually no gain), but it's bad as it's undocumented and
if I would like to use new mapi_sprop after free of source sprop, then
I've a bad luck. I think that:
a) the cast function should have one more argument, the mem_ctx which
should be used for values and arrays which require memory allocation
b) items with string and byte values should be also allocated in the
c) I'm afraid of memory usage. Are these arrays allocated on the global
memory context freed with the mapi_sprop struct or on the program
exit/global memory context free? The later is something I would like to
avoid. But I do not know how exactly this works.