kern/177759: [gpio] wrong check for unwanted flags

[Luiz Otavio O Souza]

>Number:         177759
>Category:       kern
>Synopsis:       [gpio] wrong check for unwanted flags
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Apr 10 18:00:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Luiz Otavio O Souza
>Release:        HEAD r247891
>Organization:
>Environment:
FreeBSD devel 10.0-CURRENT FreeBSD 10.0-CURRENT #7 r247891M: Wed Mar  6 10:16:45 BRT 2013     root at devel:/usr/obj/usr/src/sys/GENERIC  amd64
>Description:
clang warns about this badly written code (which got copied all over the tree):

11:47 <@dim> sys/arm/allwinner/a10_gpio.c:304:13: error: unsequenced modification and access to 'flags' [-Werror,-Wunsequenced]
11:47 <@dim>         if ((flags &= sc->sc_gpio_pins[i].gp_caps) != flags)
11:47 <@dim>                    ^                                  ~~~~~


>How-To-Repeat:

>Fix:
Apply the attached patch. It changes the check to not modify the flags variable (which isn't really needed since the function will return if flags has any invalid bit set).

Patch attached with submission follows:

Index: arm/allwinner/a10_gpio.c
===================================================================
--- arm/allwinner/a10_gpio.c	(revision 248943)
+++ arm/allwinner/a10_gpio.c	(working copy)
@@ -300,8 +300,8 @@
 	if (i >= sc->sc_gpio_npins)
 		return (EINVAL);
 
-	/* Filter out unwanted flags. */
-	if ((flags &= sc->sc_gpio_pins[i].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->sc_gpio_pins[i].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together. */
Index: arm/broadcom/bcm2835/bcm2835_gpio.c
===================================================================
--- arm/broadcom/bcm2835/bcm2835_gpio.c	(revision 248943)
+++ arm/broadcom/bcm2835/bcm2835_gpio.c	(working copy)
@@ -385,8 +385,8 @@
 	if (bcm_gpio_pin_is_ro(sc, pin))
 		return (EINVAL);
 
-	/* Filter out unwanted flags. */
-	if ((flags &= sc->sc_gpio_pins[i].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->sc_gpio_pins[i].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together. */
Index: arm/freescale/imx/imx51_gpio.c
===================================================================
--- arm/freescale/imx/imx51_gpio.c	(revision 248943)
+++ arm/freescale/imx/imx51_gpio.c	(working copy)
@@ -261,8 +261,8 @@
 	if (i >= sc->gpio_npins)
 		return (EINVAL);
 
-	/* Filter out unwanted flags */
-	if ((flags &= sc->gpio_pins[i].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->gpio_pins[i].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together */
Index: arm/xscale/ixp425/avila_gpio.c
===================================================================
--- arm/xscale/ixp425/avila_gpio.c	(revision 248943)
+++ arm/xscale/ixp425/avila_gpio.c	(working copy)
@@ -220,8 +220,8 @@
 	if (pin >= IXP4XX_GPIO_PINS || !(sc->sc_valid & mask))
 		return (EINVAL);
 
-	/* Filter out unwanted flags */
-	if ((flags &= sc->sc_pins[pin].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->sc_pins[pin].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together */
Index: arm/xscale/ixp425/cambria_gpio.c
===================================================================
--- arm/xscale/ixp425/cambria_gpio.c	(revision 248943)
+++ arm/xscale/ixp425/cambria_gpio.c	(working copy)
@@ -317,8 +317,8 @@
 	if (pin >= GPIO_PINS)
 		return (EINVAL);
 
-	/* Filter out unwanted flags */
-	if ((flags &= sc->sc_pins[pin].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->sc_pins[pin].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together */
Index: mips/atheros/ar71xx_gpio.c
===================================================================
--- mips/atheros/ar71xx_gpio.c	(revision 248950)
+++ mips/atheros/ar71xx_gpio.c	(working copy)
@@ -219,8 +219,8 @@
 	if (i >= sc->gpio_npins)
 		return (EINVAL);
 
-	/* Filter out unwanted flags */
-	if ((flags &= sc->gpio_pins[i].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->gpio_pins[i].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together */
Index: mips/cavium/octeon_gpio.c
===================================================================
--- mips/cavium/octeon_gpio.c	(revision 248943)
+++ mips/cavium/octeon_gpio.c	(working copy)
@@ -219,8 +219,8 @@
 	if (i >= sc->gpio_npins)
 		return (EINVAL);
 
-	/* Filter out unwanted flags */
-	if ((flags &= sc->gpio_pins[i].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->gpio_pins[i].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together */
Index: mips/rt305x/rt305x_gpio.c
===================================================================
--- mips/rt305x/rt305x_gpio.c	(revision 248943)
+++ mips/rt305x/rt305x_gpio.c	(working copy)
@@ -242,8 +242,8 @@
 	if (i >= sc->gpio_npins)
 		return (EINVAL);
 
-	/* Filter out unwanted flags */
-	if ((flags &= sc->gpio_pins[i].gp_caps) != flags)
+	/* Check out for unwanted flags. */
+	if ((flags & sc->gpio_pins[i].gp_caps) != flags)
 		return (EINVAL);
 
 	/* Can't mix input/output together */


>Release-Note:
>Audit-Trail:
>Unformatted: