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

[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.

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