| Message ID | 1476331103-24072-2-git-send-email-peter.hutterer@who-t.net |
|---|---|
| State | Accepted |
| Commit | 43904c9c5a0f5750a03a9bd8c96ccda182eb5a9a |
| Headers | show |
| Series |
"Series without cover letter"
( rev:
1
)
in
X.org (DEPRECATED - USE GITLAB) |
diff --git a/man/XListInputDevices.txt b/man/XListInputDevices.txt index 276660d..450f377 100644 --- a/man/XListInputDevices.txt +++ b/man/XListInputDevices.txt @@ -220,5 +220,13 @@ DESCRIPTION Floating. If the device is a master device, attached specifies the device ID of the master device this device is paired with. - To free the XDeviceInfo array created by XListInputDevices, use - XFreeDeviceList. +RETURN VALUE +------------ + + XListInputDevices returns a pointer to an array of XDeviceInfo + structs and sets ndevices_return to the number of elements in + that array. To free the XDeviceInfo array created by + XListInputDevices, use XFreeDeviceList. + + On error, XListInputDevices returns NULL and ndevices_return is + left unmodified. diff --git a/src/XListDev.c b/src/XListDev.c index e4bd3d5..dda6011 100644 --- a/src/XListDev.c +++ b/src/XListDev.c @@ -175,7 +175,7 @@ ParseClassInfo(xAnyClassPtr *any, XAnyClassPtr *Any, int num_classes) XDeviceInfo * XListInputDevices( register Display *dpy, - int *ndevices) + int *ndevices_return) { size_t s, size; xListInputDevicesReq *req; @@ -190,6 +190,7 @@ XListInputDevices( int i; unsigned long rlen; XExtDisplayInfo *info = XInput_find_display(dpy); + int ndevices; LockDisplay(dpy); if (_XiCheckExtInit(dpy, XInput_Initial_Release, info) == -1) @@ -205,8 +206,8 @@ XListInputDevices( return (XDeviceInfo *) NULL; } - if ((*ndevices = rep.ndevices)) { /* at least 1 input device */ - size = *ndevices * sizeof(XDeviceInfo); + if ((ndevices = rep.ndevices)) { /* at least 1 input device */ + size = ndevices * sizeof(XDeviceInfo); if (rep.length < (INT_MAX >> 2)) { rlen = rep.length << 2; /* multiply length by 4 */ slist = list = Xmalloc(rlen); @@ -219,17 +220,17 @@ XListInputDevices( } _XRead(dpy, (char *)list, rlen); - any = (xAnyClassPtr) ((char *)list + (*ndevices * sizeof(xDeviceInfo))); + any = (xAnyClassPtr) ((char *)list + (ndevices * sizeof(xDeviceInfo))); sav_any = any; end = (char *)list + rlen; - for (i = 0; i < *ndevices; i++, list++) { + for (i = 0; i < ndevices; i++, list++) { if(SizeClassInfo(&any, end - (char *)any, (int)list->num_classes, &s)) goto out; size += s; } Nptr = ((unsigned char *)list) + rlen; - for (i = 0, nptr = (unsigned char *)any; i < *ndevices; i++) { + for (i = 0, nptr = (unsigned char *)any; i < ndevices; i++) { if (nptr >= Nptr) goto out; size += *nptr + 1; @@ -245,10 +246,10 @@ XListInputDevices( } sclist = clist; Any = (XAnyClassPtr) ((char *)clist + - (*ndevices * sizeof(XDeviceInfo))); + (ndevices * sizeof(XDeviceInfo))); list = slist; any = sav_any; - for (i = 0; i < *ndevices; i++, list++, clist++) { + for (i = 0; i < ndevices; i++, list++, clist++) { clist->type = list->type; clist->id = list->id; clist->use = list->use; @@ -261,7 +262,7 @@ XListInputDevices( clist = sclist; nptr = (unsigned char *)any; Nptr = (unsigned char *)Any; - for (i = 0; i < *ndevices; i++, clist++) { + for (i = 0; i < ndevices; i++, clist++) { clist->name = (char *)Nptr; memcpy(Nptr, nptr + 1, *nptr); Nptr += (*nptr); @@ -270,6 +271,8 @@ XListInputDevices( } } + *ndevices_return = ndevices; + out: XFree((char *)slist); UnlockDisplay(dpy);
On 13 October 2016 at 04:58, Peter Hutterer <peter.hutterer@who-t.net> wrote: > We used to always set *ndevices to the number of devices returned by the > server. This magically worked because we pretty much never returned an error > except on faulty server or library implementations. With 19a9cd60 we now have > more chances of getting an error, so the polite thing is to just leave *ndevices > alone when we error out. > > Document it as such in the man page, just in case someone accidentally reads > it. > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> > CC: Niels Ole Salscheider <niels_ole@salscheider-online.de> > --- > Changes to v1: > - Niels' first patch set ndevices to 0, this one leaves it untouched > Slightly split between "doing the right thing" and "the cat is out of the bag" ;-) I'm leaning towards the former, although we might want to prod Chromium devs and/or send them a patch ? Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> -Emil
On Fri, Oct 14, 2016 at 02:28:55PM +0100, Emil Velikov wrote: > On 13 October 2016 at 04:58, Peter Hutterer <peter.hutterer@who-t.net> wrote: > > We used to always set *ndevices to the number of devices returned by the > > server. This magically worked because we pretty much never returned an error > > except on faulty server or library implementations. With 19a9cd60 we now have > > more chances of getting an error, so the polite thing is to just leave *ndevices > > alone when we error out. > > > > Document it as such in the man page, just in case someone accidentally reads > > it. > > > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> > > CC: Niels Ole Salscheider <niels_ole@salscheider-online.de> > > --- > > Changes to v1: > > - Niels' first patch set ndevices to 0, this one leaves it untouched > > > Slightly split between "doing the right thing" and "the cat is out of > the bag" ;-) I don't think the cat is out of the bag anyway here. ndevices was *always* wrong in case of error. either it was untouched or set to the list of devices even though NULL was returned. the only reason this worked is because we never had an error. the cat remains thus firmly packaged, if (as usual) in an unclear state of vividness. > I'm leaning towards the former, although we might want to prod > Chromium devs and/or send them a patch ? the chromium code is broken, it cannot handle *any* error case. on the first call, the devices list is NULL and count is 0. XListInputDevices is fails, we currently get a NULL list but a count of != 0. Which will then crash when looping through the list and dereferencing the nonexistent members. At least with this fix, count stays on 0 and while XListInputDevices will get called every time, everything else should simply skip over any loop over the devices then (since count remains at 0). anyway, I just tried to file a bug, but "You need a Google Account associated with your email address in order to use the bug system." so there goes that idea. so now I'm just CC-ing the three most recent @chromium.org addresses from xorg-devel and cross my fingers and hope :) Cheers, Peter
+sadrul On Mon, Oct 17, 2016 at 1:43 PM, Peter Hutterer <peter.hutterer@who-t.net> wrote: > On Fri, Oct 14, 2016 at 02:28:55PM +0100, Emil Velikov wrote: >> On 13 October 2016 at 04:58, Peter Hutterer <peter.hutterer@who-t.net> wrote: >> > We used to always set *ndevices to the number of devices returned by the >> > server. This magically worked because we pretty much never returned an error >> > except on faulty server or library implementations. With 19a9cd60 we now have >> > more chances of getting an error, so the polite thing is to just leave *ndevices >> > alone when we error out. >> > >> > Document it as such in the man page, just in case someone accidentally reads >> > it. >> > >> > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> >> > CC: Niels Ole Salscheider <niels_ole@salscheider-online.de> >> > --- >> > Changes to v1: >> > - Niels' first patch set ndevices to 0, this one leaves it untouched >> > >> Slightly split between "doing the right thing" and "the cat is out of >> the bag" ;-) > > I don't think the cat is out of the bag anyway here. ndevices was *always* > wrong in case of error. either it was untouched or set to the list of > devices even though NULL was returned. the only reason this worked is > because we never had an error. the cat remains thus firmly packaged, if (as > usual) in an unclear state of vividness. > >> I'm leaning towards the former, although we might want to prod >> Chromium devs and/or send them a patch ? > > the chromium code is broken, it cannot handle *any* error case. on the first > call, the devices list is NULL and count is 0. XListInputDevices is > fails, we currently get a NULL list but a count of != 0. Which > will then crash when looping through the list and dereferencing the > nonexistent members. At least with this fix, count stays on 0 and while > XListInputDevices will get called every time, everything else > should simply skip over any loop over the devices then (since count remains > at 0). > > anyway, I just tried to file a bug, but "You need a Google Account > associated with your email address in order to use the bug system." so there > goes that idea. so now I'm just CC-ing the three most recent @chromium.org > addresses from xorg-devel and cross my fingers and hope :) Filed https://bugs.chromium.org/p/chromium/issues/detail?id=656506, thanks for reporting! Best, Nicolas
We used to always set *ndevices to the number of devices returned by the server. This magically worked because we pretty much never returned an error except on faulty server or library implementations. With 19a9cd60 we now have more chances of getting an error, so the polite thing is to just leave *ndevices alone when we error out. Document it as such in the man page, just in case someone accidentally reads it. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> CC: Niels Ole Salscheider <niels_ole@salscheider-online.de> --- Changes to v1: - Niels' first patch set ndevices to 0, this one leaves it untouched man/XListInputDevices.txt | 12 ++++++++++-- src/XListDev.c | 21 ++++++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-)