Re: [Linuxwacom-devel] [PATCH] If _source is NULL, don't auto-pick the type.
On Wed, Oct 24, 2012 at 10:29 PM, Ping Cheng pingli...@gmail.com wrote: On Wednesday, October 24, 2012, Jason Gerecke wrote: On Wed, Oct 24, 2012 at 12:20 AM, Peter Hutterer peter.hutte...@who-t.net wrote: An xorg.conf InputDevice section that does not have Option Type set is invalid. Skip the type assignment and return, triggering an error about an invalid type lateron. I think the above comments reflect a valid reaction of the driver to an invalid xorg.conf section. The part that I miss is how to link it to the description below. Fixes crash triggered as of xf86-input-wacom-0.17.0-8-g0debde6 by having an InputDevice section like this: Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection This would lead to the device being assigned a type and assumed to be for hotplugging. Later, when the device attributes are duplicated during QueueHotplug, asprintf() tries to duplicate attr-product, which is always NULL for xorg.conf devices. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Full config for the lazy :) Section ServerLayout Identifier Dummy layout Option AutoAddDevices off Screen 0 dummy screen 0 0 InputDevice --device-- EndSection Section ServerFlags Option Log flush EndSection Section Device Identifier dummy Driver dummy EndSection Section Screen Identifier dummy screen Device dummy EndSection Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection Running 9f896f4800593c7fa232e40fd934b7f51d0dde6d (0.17.99.1) with that config file below crashes the server immediately. git bisect pointed to xf86-input-wacom-0.17.0-8-g0debde6, but that seems to just trigger the bug, it's not the direct cause of it. src/wcmValidateDevice.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c index 658092b..bc5ad44 100644 --- a/src/wcmValidateDevice.c +++ b/src/wcmValidateDevice.c @@ -578,6 +578,9 @@ int wcmNeedAutoHotplug(InputInfoPtr pInfo, const char **type) if (*type) /* type specified, don't hotplug */ return 0; + if (!source) /* xorg.conf device, don't auto-pick type */ + return 0; + I am too lazy to read the whole routine/code. I guess this dummy --device-- will be ignored and some message indicating invalid xorg.conf options will be posted. Then, a solution to make the device usable for the dummy user would be to add it through hot plugging. Or, maybe we do not want to make the dummy feel good :)? I wondered the same thing initially too. After the function returns zero to wcmConfig.c:wcmPreInit(), the next line calls wcmIsAValidType, which fails because NULL isn't a valid type. The error message is printed, and the if statement the call is embedded in is satisfied since we have a statically-defined device with a bad type. This whisks us off to SetupProc_fail to keep us from bombing out later on. Jason --- When you're rife with devastation / There's a simple explanation: You're a toymaker's creation / Trapped inside a crystal ball. And whichever way he tilts it / Know that we must be resilient We won't let them break our spirits / As we sing our silly song. Ping if (source strcmp(source, server/hal) strcmp(source, server/udev)) return 0; L -- 1.7.11.7 -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
[Linuxwacom-devel] [PATCH] If _source is NULL, don't auto-pick the type.
An xorg.conf InputDevice section that does not have Option Type set is invalid. Skip the type assignment and return, triggering an error about an invalid type lateron. Fixes crash triggered as of xf86-input-wacom-0.17.0-8-g0debde6 by having an InputDevice section like this: Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection This would lead to the device being assigned a type and assumed to be for hotplugging. Later, when the device attributes are duplicated during QueueHotplug, asprintf() tries to duplicate attr-product, which is always NULL for xorg.conf devices. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Full config for the lazy :) Section ServerLayout Identifier Dummy layout Option AutoAddDevices off Screen 0 dummy screen 0 0 InputDevice --device-- EndSection Section ServerFlags Option Log flush EndSection Section Device Identifier dummy Driver dummy EndSection Section Screen Identifier dummy screen Device dummy EndSection Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection Running 9f896f4800593c7fa232e40fd934b7f51d0dde6d (0.17.99.1) with that config file below crashes the server immediately. git bisect pointed to xf86-input-wacom-0.17.0-8-g0debde6, but that seems to just trigger the bug, it's not the direct cause of it. src/wcmValidateDevice.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c index 658092b..bc5ad44 100644 --- a/src/wcmValidateDevice.c +++ b/src/wcmValidateDevice.c @@ -578,6 +578,9 @@ int wcmNeedAutoHotplug(InputInfoPtr pInfo, const char **type) if (*type) /* type specified, don't hotplug */ return 0; + if (!source) /* xorg.conf device, don't auto-pick type */ + return 0; + if (source strcmp(source, server/hal) strcmp(source, server/udev)) return 0; -- 1.7.11.7 -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH] If _source is NULL, don't auto-pick the type.
On Wed, Oct 24, 2012 at 12:20 AM, Peter Hutterer peter.hutte...@who-t.net wrote: An xorg.conf InputDevice section that does not have Option Type set is invalid. Skip the type assignment and return, triggering an error about an invalid type lateron. Fixes crash triggered as of xf86-input-wacom-0.17.0-8-g0debde6 by having an InputDevice section like this: Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection This would lead to the device being assigned a type and assumed to be for hotplugging. Later, when the device attributes are duplicated during QueueHotplug, asprintf() tries to duplicate attr-product, which is always NULL for xorg.conf devices. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net --- Full config for the lazy :) Section ServerLayout Identifier Dummy layout Option AutoAddDevices off Screen 0 dummy screen 0 0 InputDevice --device-- EndSection Section ServerFlags Option Log flush EndSection Section Device Identifier dummy Driver dummy EndSection Section Screen Identifier dummy screen Device dummy EndSection Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection Running 9f896f4800593c7fa232e40fd934b7f51d0dde6d (0.17.99.1) with that config file below crashes the server immediately. git bisect pointed to xf86-input-wacom-0.17.0-8-g0debde6, but that seems to just trigger the bug, it's not the direct cause of it. src/wcmValidateDevice.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c index 658092b..bc5ad44 100644 --- a/src/wcmValidateDevice.c +++ b/src/wcmValidateDevice.c @@ -578,6 +578,9 @@ int wcmNeedAutoHotplug(InputInfoPtr pInfo, const char **type) if (*type) /* type specified, don't hotplug */ return 0; + if (!source) /* xorg.conf device, don't auto-pick type */ + return 0; + if (source strcmp(source, server/hal) strcmp(source, server/udev)) return 0; -- 1.7.11.7 Reviewed-by: Jason Gerecke killert...@gmail.com Jason --- When you're rife with devastation / There's a simple explanation: You're a toymaker's creation / Trapped inside a crystal ball. And whichever way he tilts it / Know that we must be resilient We won't let them break our spirits / As we sing our silly song. -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH] If _source is NULL, don't auto-pick the type.
On Wednesday, October 24, 2012, Jason Gerecke wrote: On Wed, Oct 24, 2012 at 12:20 AM, Peter Hutterer peter.hutte...@who-t.net javascript:; wrote: An xorg.conf InputDevice section that does not have Option Type set is invalid. Skip the type assignment and return, triggering an error about an invalid type lateron. I think the above comments reflect a valid reaction of the driver to an invalid xorg.conf section. The part that I miss is how to link it to the description below. Fixes crash triggered as of xf86-input-wacom-0.17.0-8-g0debde6 by having an InputDevice section like this: Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection This would lead to the device being assigned a type and assumed to be for hotplugging. Later, when the device attributes are duplicated during QueueHotplug, asprintf() tries to duplicate attr-product, which is always NULL for xorg.conf devices. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net javascript:; --- Full config for the lazy :) Section ServerLayout Identifier Dummy layout Option AutoAddDevices off Screen 0 dummy screen 0 0 InputDevice --device-- EndSection Section ServerFlags Option Log flush EndSection Section Device Identifier dummy Driver dummy EndSection Section Screen Identifier dummy screen Device dummy EndSection Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection Running 9f896f4800593c7fa232e40fd934b7f51d0dde6d (0.17.99.1) with that config file below crashes the server immediately. git bisect pointed to xf86-input-wacom-0.17.0-8-g0debde6, but that seems to just trigger the bug, it's not the direct cause of it. src/wcmValidateDevice.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c index 658092b..bc5ad44 100644 --- a/src/wcmValidateDevice.c +++ b/src/wcmValidateDevice.c @@ -578,6 +578,9 @@ int wcmNeedAutoHotplug(InputInfoPtr pInfo, const char **type) if (*type) /* type specified, don't hotplug */ return 0; + if (!source) /* xorg.conf device, don't auto-pick type */ + return 0; + I am too lazy to read the whole routine/code. I guess this dummy --device-- will be ignored and some message indicating invalid xorg.conf options will be posted. Then, a solution to make the device usable for the dummy user would be to add it through hot plugging. Or, maybe we do not want to make the dummy feel good :)? Ping if (source strcmp(source, server/hal) strcmp(source, server/udev)) return 0; L -- 1.7.11.7 -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH] If _source is NULL, don't auto-pick the type.
On Wed, Oct 24, 2012 at 10:29:59PM -0700, Ping Cheng wrote: On Wednesday, October 24, 2012, Jason Gerecke wrote: On Wed, Oct 24, 2012 at 12:20 AM, Peter Hutterer peter.hutte...@who-t.net javascript:; wrote: An xorg.conf InputDevice section that does not have Option Type set is invalid. Skip the type assignment and return, triggering an error about an invalid type lateron. I think the above comments reflect a valid reaction of the driver to an invalid xorg.conf section. The part that I miss is how to link it to the description below. The above is the behaviour after applying the patch. Without the patch, the behaviour is to crash. Which admittedly is a strong hint that something isn't right with the config, but not verbose enough to point the user at what they've done wrong :) Fixes crash triggered as of xf86-input-wacom-0.17.0-8-g0debde6 by having an InputDevice section like this: Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection This would lead to the device being assigned a type and assumed to be for hotplugging. Later, when the device attributes are duplicated during QueueHotplug, asprintf() tries to duplicate attr-product, which is always NULL for xorg.conf devices. Signed-off-by: Peter Hutterer peter.hutte...@who-t.net javascript:; --- Full config for the lazy :) Section ServerLayout Identifier Dummy layout Option AutoAddDevices off Screen 0 dummy screen 0 0 InputDevice --device-- EndSection Section ServerFlags Option Log flush EndSection Section Device Identifier dummy Driver dummy EndSection Section Screen Identifier dummy screen Device dummy EndSection Section InputDevice Identifier --device-- Driver wacom Option CorePointer on EndSection Running 9f896f4800593c7fa232e40fd934b7f51d0dde6d (0.17.99.1) with that config file below crashes the server immediately. git bisect pointed to xf86-input-wacom-0.17.0-8-g0debde6, but that seems to just trigger the bug, it's not the direct cause of it. src/wcmValidateDevice.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/wcmValidateDevice.c b/src/wcmValidateDevice.c index 658092b..bc5ad44 100644 --- a/src/wcmValidateDevice.c +++ b/src/wcmValidateDevice.c @@ -578,6 +578,9 @@ int wcmNeedAutoHotplug(InputInfoPtr pInfo, const char **type) if (*type) /* type specified, don't hotplug */ return 0; + if (!source) /* xorg.conf device, don't auto-pick type */ + return 0; + I am too lazy to read the whole routine/code. I guess this dummy --device-- will be ignored and some message indicating invalid xorg.conf options will be posted. Then, a solution to make the device usable for the dummy user would be to add it through hot plugging. Or, maybe we do not want to make the dummy feel good :)? Option Type has always been required for xorg.conf configured devices, we don't hotplug those on the assumption that a user that only adds, say, stylus + eraser (but not pad) to the xorg.conf has a valid reason to do so. The problem was that having a device _without_ Option Type would crash the server. With that patch it leaves the type as NULL and the driver will then complain no valid type set Cheers, Peter Ping if (source strcmp(source, server/hal) strcmp(source, server/udev)) return 0; L -- 1.7.11.7 -- Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_sfd2d_oct ___ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
