Am 10.10.2016 12:24, schrieb Emil Velikov: > On 9 October 2016 at 21:31, Niels Ole Salscheider > <[email protected]> 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 < >>> >>> [email protected]> wrote: >>>> Catch the error case separately. This fixes a few crashes on my computer. >>>> >>>> Signed-off-by: Niels Ole Salscheider <[email protected] >>>> <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 > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
