Re: [Libvncserver-common] Memory Leak

[Johann O. <joh...@si...>]

Hi Johannes,


Am 17.03.2015 um 12:10 schrieb Johannes Schindelin:
> Index: git/libvncserver/main.c
>> ===================================================================
>> --- git.orig/libvncserver/main.c    2015-01-15 14:56:56.538094757 +0100
>> +++ git/libvncserver/main.c    2015-01-15 15:04:53.000000000 +0100
>> @@ -1020,7 +1020,7 @@
>>    FREE_IF(colourMap.data.bytes);
>>    FREE_IF(underCursorBuffer);
>>    TINI_MUTEX(screen->cursorMutex);
>> -  if(screen->cursor && screen->cursor->cleanup)
>> +  if(screen->cursor)
>>      rfbFreeCursor(screen->cursor);
>>
>> #ifdef LIBVNCSERVER_HAVE_LIBZ
>
> This patch is incorrect because the `cleanup` flag is an indicator 
> whether the `cursor` attribute points to `malloc()`ed data. Consider 
> this code:
>
> ```c
> struct rfbCursor cursor = {
>   FALSE, FALSE, FALSE, FALSE,
>   "\xff", NULL, NULL,
>   1, 1, 0, 0,
>   0xffff, 0xffff, 0xffff,
>   0, 0, 0,
>   NULL, NULL,
>   FALSE
> };
> screen->cursor = &cursor;
> ```
>
> With your patch, this code would all of a sudden crash in a nasty way, 
> even if it is correct.
>
> Ciao,
> Johannes
>
the patch is correct, because in the function rfbFreeCursor the cleanup 
flag is checked. And all other (mask, source, ..)
will be free correct.

Regards
   Johann