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

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

[Simon Budig]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

-----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
       simon.budig@kernelconcepts.de      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-----

[-- Attachment #2: input-fix-macros.patch --]
[-- Type: text/x-patch, Size: 2879 bytes --]

commit c88a91cb31a158a5e993484d91a0634a2aab1a43
Author: Simon Budig <simon@budig.de>
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 <simon.budig@kernelconcepts.de>

diff --git a/include/linux/input.h b/include/linux/input.h
index 9e5393d..31dd2b8 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -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 */

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

[Simon Budig]

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

On 06/01/2011 11:20 PM, Simon Budig wrote:
> 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
       simon.budig@kernelconcepts.de      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-----

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

[Dmitry Torokhov]

Hi Simon,

On Tue, Jun 07, 2011 at 12:51:12PM +0200, Simon Budig wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 06/01/2011 11:20 PM, Simon Budig wrote:
> > 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.

-- 
Dmitry