The SDL forums have moved to discourse.libsdl.org.
This is just a read-only archive of the previous forums, to keep old links working.

On regular 64-bits Linux (doesn't seem to be a problem on 32-bit
Linux), normal run of SDL tries to use the Linux input events (for
touch, it seems), but fails because it doesn't have permissions.

This shouldn't be a problem, but there is no check, besides an
annoying printf() message, that the open() call failed.
The result is the standard output being bombarded with "Error:
Couldn't open stream" messages (file: src/video/x11/SDL_x11events.c
line:562).

This by itself is wrong, because after printing the message it happily
goes to use the invalid file descriptor on reads(), but the real
problem is that there is no check on the open() call that it failed.

The first patch is just indentation fixes (mixed tabs and spaces),
while the second patch just adds a check for the open() result and
ignores the device if it fails.


Regards,
~Nuno Lucas

_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

On Tuesday 27 September 2011 21:00:31 Nuno Lucas wrote:

Hi Nuno (and anyone with code check-in privilege),

Even though I'm not running 64-bit Linux and am not using SDL 1.3, I took a
look at your 2nd patch and the 'abs[5]' line caught my eye.

By sheer coincidence, I had just posted a bugfix for a problem in the SDL
1.2.14 joystick property detection code which I suspected was related to the
'abs[5] definition that I noticed in your patch. Here's my post:

http://forums.libsdl.org/viewtopic.php?t=7593

On further investigation, I found that the SDL 1.3 (Aug 05 2011 snapshot, aka
SDL1.3.0-5605) X11 "touch" interface code has the _exact_ _same_ _bug_. In
essence, it only allocates 5 elements for the 'ioctl(.., EVIOCGABS(..), ..)'
call, which actually returns 6 elements, as defined in '/usr/linux/input.h'
(Slackware 13.1):

struct input_absinfo {
__s32 value;
__s32 minimum;
__s32 maximum;
__s32 fuzz;
__s32 flat;
__s32 resolution;
};

Obviously this 'ioctl()' call with the inadequate storage area will corrupt
memory and probably introduce some undesirable behavior, much like it did in
the 1.2.14 library's Linux joystick detection, whereby it was only detecting 1
axis (not 3) and 0 hat switches (not 1) for my joystick.

To anyone with check-in privileges: The offending line in the 1.3 library is in
the same file that Nuno patched (src/video/x11/SDL_x11touch.c). His patch just
happened to "touch the edges" of this code (i.e. but didn't actually change
it).

Here's the problem:

int abs[5];
ioctl(data->eventStream,EVIOCGABS(0),abs);

A proper fix would be one like the fix that was done to the SDL 1.3 joystick
code, using 'struct input_absinfo absinfo;' instead of 'int abs[5];', also
correcting the variable used on all 3 calls to 'ioctl(.., EVIOCGABS(..), ..)'.
But even a simple change of array size from 5 to 6 would suffice in a pinch,
just as it did for me with the joystick properties detection bug in SDL
1.2.14.

Hopefully, this report will eliminate one more bug from 1.3.x code.

A couple of closing notes: (1) For anyone unaware, there is no "touch"
interface code in SDL 1.2.x, so Nuno's patches and my addendum fix is not
applicable there. (2) Given this repeated problem with inadequate allocation
for the EVIOCGABS ioctl, I took a look through all of the 1.2.14 and
1.3.0-5605 SDL source code and, fortunately, the Linux joystick and X11
"touch" code are the only areas affected.

Regards,
Bill Marr
_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

Hello,

On Wed, Sep 28, 2011 at 17:54, Marr wrote:

After seeing the "quality" of the "touch/joystick" patch this doesn't
surprise me at all.
Besides the blatantly wrong indentation, all the code seems a huge
hack, without any regard for good programming practices.
If it was me, I would revert the whole patch until properly reviewed,
as I'm sure more bugs are waiting silently.

For one, I don't understand why the direct open of Linux input events
when X is already running. That should be a X job. It should only be
necessary when X is not running, like with the fbdev driver (or
possibly with a new wayland driver).

I don't know nothing about joysticks, but at least for touch events
there is no reason no not use the X input subsystem.

[...]

Regards,
~Nuno Lucas

_______________________________________________
SDL mailing list

http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

This is in, thanks!

On Tue, Sep 27, 2011 at 9:00 PM, Nuno Lucas wrote: