[PATCH libXi] Allocate enough memory for raw events + extra data.

[Jeremy Huddleston]

On May 2, 2011, at 10:00 PM, Peter Hutterer wrote:

> On Mon, May 02, 2011 at 09:05:21PM -0700, Jeremy Huddleston wrote:
>> Woot.  That takes care of:
>> 
>> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-kzyPla.html#EndPath
>> 
>> It looks like there are others you might want to do at the same time:
>> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/
> 
> re "Idempotent operation"
> I'm staring hard at those but don't really see the issue.
> SetReqLen(req, foo, foo); is valid and for any foo that is non-zero
> shouldn't have the effect of "Assigned value is always the same as the
> existing value". SetReqLen essentially does req->length += foo.

Yeah... "within expansion of macro..." results are a bit bad with clang.  I just opened a bug about that.  Hopefully future analytics will be better.  For now, at worse it's a dead assignment, and we can come back to it when clang gets better at printing those results.

> 
> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-yDDyYj.html#EndPath
> "undefined allocation of 0 bytes" would be a server bug afaict

I dunno... is there anywhere that we guarantee that Xmalloc(0) returns NULL?  If so, then the macro should probably be ((size) > 0 ? malloc(size) : NULL) ... hopefully there is nothing actually using side-effects inside of Xmalloc(...) ...

> http://people.freedesktop.org/~jeremyhu/analyzer/yuffie/20110429-1617/libXi/report-n6GgtE.html#EndPath
> don't want to change this one, it's there for symmetry.

Would you mind doing this to shut up the warning?  I know it's ugly, but there unfortunately isn't a better way to whitelist in the analyzer yet:

#ifndef __clang_analyzer__
...
#endif


> 
> Cheers,
>  Peter
> 
>> On May 2, 2011, at 8:50 PM, Peter Hutterer wrote:
>> 
>>> Necessary space was calculated, but not actually used to allocate memory. As
>>> a result, valuator data would overwrite the allocated memory.
>>> 
>>> ==4166== Invalid write of size 1
>>> ==4166==    at 0x4C29F04: memcpy (mc_replace_strmem.c:497)
>>> ==4166==    by 0x8F39180: ??? (in /usr/lib/libXi.so.6.1.0)
>>> ==4166==    by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==    by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==    by 0x49C3E3: process_key (x11_be.c:1065)
>>> ==4166==    by 0x49EA5C: event_key_release (x11_be.c:2201)
>>> ==4166==    by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
>>> ==4166==    by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
>>> ==4166==    by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
>>> ==4166==    by 0x87549C9: start_thread (pthread_create.c:300)
>>> ==4166==    by 0x8A516FC: clone (clone.S:112)
>>> ==4166==  Address 0x168afe80 is 0 bytes after a block of size 96 alloc'd
>>> ==4166==    at 0x4C284A8: malloc (vg_replace_malloc.c:236)
>>> ==4166==    by 0x8F390BD: ??? (in /usr/lib/libXi.so.6.1.0)
>>> ==4166==    by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==    by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
>>> ==4166==    by 0x49C3E3: process_key (x11_be.c:1065)
>>> ==4166==    by 0x49EA5C: event_key_release (x11_be.c:2201)
>>> ==4166==    by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
>>> ==4166==    by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
>>> ==4166==    by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
>>> ==4166==    by 0x87549C9: start_thread (pthread_create.c:300)
>>> 
>>> Reported-by: Roger Cruz <roger.cruz at virtualcomputer.com>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>> Sorry, was away for a number of days and only got to this now. You're right,
>>> it's a bug, copy/paste I guess.
>>> 
>>> src/XExtInt.c |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>> 
>>> diff --git a/src/XExtInt.c b/src/XExtInt.c
>>> index d1451cc..134ccc6 100644
>>> --- a/src/XExtInt.c
>>> +++ b/src/XExtInt.c
>>> @@ -1259,7 +1259,7 @@ copyRawEvent(XGenericEventCookie *cookie_in,
>>>    len = sizeof(XIRawEvent) + in->valuators.mask_len;
>>>    len += bits * sizeof(double) * 2;
>>> 
>>> -    ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
>>> +    ptr = cookie_out->data = malloc(len);
>>>    if (!ptr)
>>>        return False;
>>> 
>>> -- 
>>> 1.7.4.4
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>