Skip to contentSkip to conversation

Patch to add guarding parentheses to some macros in linux/input.h

PATCH Simon Budig 2 participants 3 messages Jun 1, 2011 – Jul 5, 2011

Subsystems (2)
HID Input drivers
lore
Share & tools
lore

Simon Budig

author
#0 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi all.

This is a patch to add guarding parentheses around some macro argument uses. This is good practice to avoid pitfalls like this:

ioctl (fd, EVIOCGABS (have_mt ? ABS_MT_POSITION_X : ABS_X), &abs);

Which currently totally does not do what is expected.

The attached patch adds parentheses to ensure correct operator precedence.

Please consider this for inclusion into the Linux Kernel.

Thanks,
Simon
- --

       Simon Budig                        kernel concepts GbR
       [email protected]      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3mrToACgkQO2O/RXesiHBWcQCfdYFhFgKGaxvE5szwIJe3vfNO yy4AoJssXo0zvSYeVJqA/ViOVlq/ITyj
=jtQG
-----END PGP SIGNATURE-----

commit c88a91cb31a158a5e993484d91a0634a2aab1a43

Author: Simon Budig <[email protected]>
Date:   Wed Jun 1 23:04:42 2011 +0200

input: add guarding parentheses to macros

Put parentheses around macro argument uses. This avoids pitfalls for the programmer, where the argument expansion does not give the expected result.

Signed-off-by Simon Budig <[email protected]>
include/linux/input.h +13 -13
index 9e5393d..31dd2b8 100644
@@ -109,19 +109,19 @@ struct input_keymap_entry {
 #define EVIOCSKEYCODE		_IOW('E', 0x04, unsigned int[2])        /* set keycode */
 #define EVIOCSKEYCODE_V2	_IOW('E', 0x04, struct input_keymap_entry)
 
-#define EVIOCGNAME(len)		_IOC(_IOC_READ, 'E', 0x06, len)		/* get device name */
-#define EVIOCGPHYS(len)		_IOC(_IOC_READ, 'E', 0x07, len)		/* get physical location */
-#define EVIOCGUNIQ(len)		_IOC(_IOC_READ, 'E', 0x08, len)		/* get unique identifier */
-#define EVIOCGPROP(len)		_IOC(_IOC_READ, 'E', 0x09, len)		/* get device properties */
-
-#define EVIOCGKEY(len)		_IOC(_IOC_READ, 'E', 0x18, len)		/* get global key state */
-#define EVIOCGLED(len)		_IOC(_IOC_READ, 'E', 0x19, len)		/* get all LEDs */
-#define EVIOCGSND(len)		_IOC(_IOC_READ, 'E', 0x1a, len)		/* get all sounds status */
-#define EVIOCGSW(len)		_IOC(_IOC_READ, 'E', 0x1b, len)		/* get all switch states */
-
-#define EVIOCGBIT(ev,len)	_IOC(_IOC_READ, 'E', 0x20 + ev, len)	/* get event bits */
-#define EVIOCGABS(abs)		_IOR('E', 0x40 + abs, struct input_absinfo)	/* get abs value/limits */
-#define EVIOCSABS(abs)		_IOW('E', 0xc0 + abs, struct input_absinfo)	/* set abs value/limits */
+#define EVIOCGNAME(len)		_IOC(_IOC_READ, 'E', 0x06, (len))	/* get device name */
+#define EVIOCGPHYS(len)		_IOC(_IOC_READ, 'E', 0x07, (len))	/* get physical location */
+#define EVIOCGUNIQ(len)		_IOC(_IOC_READ, 'E', 0x08, (len))	/* get unique identifier */
+#define EVIOCGPROP(len)		_IOC(_IOC_READ, 'E', 0x09, (len))	/* get device properties */
+
+#define EVIOCGKEY(len)		_IOC(_IOC_READ, 'E', 0x18, (len))	/* get global key state */
+#define EVIOCGLED(len)		_IOC(_IOC_READ, 'E', 0x19, (len))	/* get all LEDs */
+#define EVIOCGSND(len)		_IOC(_IOC_READ, 'E', 0x1a, (len))	/* get all sounds status */
+#define EVIOCGSW(len)		_IOC(_IOC_READ, 'E', 0x1b, (len))	/* get all switch states */
+
+#define EVIOCGBIT(ev,len)	_IOC(_IOC_READ, 'E', 0x20 + (ev), (len))	/* get event bits */
+#define EVIOCGABS(abs)		_IOR('E', 0x40 + (abs), struct input_absinfo)	/* get abs value/limits */
+#define EVIOCSABS(abs)		_IOW('E', 0xc0 + (abs), struct input_absinfo)	/* set abs value/limits */
 
 #define EVIOCSFF		_IOC(_IOC_WRITE, 'E', 0x80, sizeof(struct ff_effect))	/* send a force effect to a force feedback device */
 #define EVIOCRMFF		_IOW('E', 0x81, int)			/* Erase a force effect */
author
#1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Simon Budig

The attached patch adds parentheses to ensure correct operator precedence.

Seems this patch did not draw any attention to it. Is this because it is somewhat pointless? Or is it too uncontroversial? :-)

I'd appreciate any hints on how to proceed from here.

Thanks,
Simon

- --

       Simon Budig                        kernel concepts GbR
       [email protected]      Sieghuetter Hauptweg 48
       +49-271-771091-17                  D-57072 Siegen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3uAqAACgkQO2O/RXesiHD8bwCfZz10ADGqx0yUbPJCSlmvNthL JZsAn2Mc+gj1UQ5nSAWGonf46s6Xg/Uq
=Ug6H
-----END PGP SIGNATURE-----

maintainer
#2 lore raw

Hi Simon,

Quoted 11 lines · Simon Budig

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Simon Budig

The attached patch adds parentheses to ensure correct operator precedence.

Seems this patch did not draw any attention to it. Is this because it is somewhat pointless? Or is it too uncontroversial? :-)

I'd appreciate any hints on how to proceed from here.

I do not believe that 'len' will give any trouble since there are no computations done with this macro argument, but you are absolutely correct that 'ev' and 'abs' should be protected in EVIOCGBIT and EVIOC(G|S)ABS. I extracted the relevant changes from your patch and queued for 3.1.

Thanks.

Signature · 1 line
Dmitry