[PATCH libXi] SizeClassInfo can return 0 even without an error

[walter harms]

Am 10.10.2016 12:24, schrieb Emil Velikov:
> On 9 October 2016 at 21:31, Niels Ole Salscheider
> <niels_ole at salscheider-online.de> wrote:
>> Hi Emil,
>>
>> On Sunday, 9 October 2016, 15:34:28 CEST, Emil Velikov wrote:
>>> Hi Niels,
>>>
>>> On Friday, 7 October 2016, Niels Ole Salscheider <
>>>
>>> niels_ole at salscheider-online.de> wrote:
>>>> Catch the error case separately. This fixes a few crashes on my computer.
>>>>
>>>> Signed-off-by: Niels Ole Salscheider <niels_ole at salscheider-online.de
>>>> <javascript:;>>
>>>> ---
>>>>
>>>>  src/XListDev.c | 21 ++++++++++-----------
>>>>  1 file changed, 10 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/src/XListDev.c b/src/XListDev.c
>>>> index f850cd0..d0c6bf2 100644
>>>> --- a/src/XListDev.c
>>>> +++ b/src/XListDev.c
>>>> @@ -73,27 +73,27 @@ static int pad_to_xid(int base_size)
>>>>
>>>>      return ((base_size + padsize - 1)/padsize) * padsize;
>>>>
>>>>  }
>>>>
>>>> -static size_t
>>>> -SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes)
>>>> +static int
>>>> +SizeClassInfo(xAnyClassPtr *any, size_t len, int num_classes, size_t
>>>> *size)
>>>>
>>>>  {
>>>>
>>>> -    int size = 0;
>>>>
>>>>      int j;
>>>>
>>>> +    *size = 0;
>>>
>>> No function should alter the contents of the arguments in case of an error.
>>> For your other libXi patch one might want to fix the callers, if applicable.
>>>
>>> If possible please mention a bug report/link or a bit more about how you
>>> hit this. Wondering how it has gone unnoticed for so long.
>>
>> I encountered the bug in chromium and all users of it, including all
>> applications that use QtWebEngine. I now hit the error path because of the bug
>> that is fixed by this patch.
>>
>> Chromium crashes in the following lines: https://chromium.googlesource.com/
>> chromium/src/+/master/ui/events/devices/x11/device_data_manager_x11.cc#246
>> Here, GetXDeviceList calls XListInputDevices:
>> https://chromium.googlesource.com/chromium/src/+/master/ui/events/devices/x11/
>> device_list_cache_x11.cc#53
>>
>> The chromium implementation is only correct if ndevices is set to 0 in the
>> error case since it does not check if a null pointer is returned. I was not
>> sure if it is supposed to do the latter since the man page for
>> XListInputDevices doesn't mention it.
>>
> Eeek, the man page does not mention anything, indeed. So even if one
> updates/corrects the man page there'll likely be other users which
> depend on ndevices being 0.

but someone should fix the man-page. NTL it is common practice to assume
that variables are not proper initialized if the function that initialize
returns any error.

re,
 wh

> 
> That said, you can drop the odd(?) practise from this patch and use it
> only in the latter ? Please mention your findings (above) in the
> commit message/patch body.
> 
> Thanks
> Emil
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel