On Thu, Mar 09, 2017 at 09:30:40PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 10, 2017 at 07:08:28AM +1100, Tobin C. Harding wrote:
> > On Thu, Mar 09, 2017 at 02:04:20PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Mar 09, 2017 at 03:47:05PM +1100, Tobin C. Harding wrote:
> > > > Checkpatch emits CHECK: Unnecessary parentheses.
> > > >
> > > > Remove unnecessary parentheses.
> > > >
> > > > Signed-off-by: Tobin C. Harding <me@xxxxxxxx>
> > > > ---
> > > > drivers/staging/ks7010/ks_hostif.c | 24 ++++++++++++------------
> > > > 1 file changed, 12 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/ks7010/ks_hostif.c b/drivers/staging/ks7010/ks_hostif.c
> > > > index a354e34e..b75ef1d6 100644
> > > > --- a/drivers/staging/ks7010/ks_hostif.c
> > > > +++ b/drivers/staging/ks7010/ks_hostif.c
> > > > @@ -36,7 +36,7 @@ inline u8 get_BYTE(struct ks_wlan_private *priv)
> > > > {
> > > > u8 data;
> > > >
> > > > - data = *(priv->rxp)++;
> > > > + data = *priv->rxp++;
> > >
> > > Are you sure this is ok? I would have to dig out a book to find the
> > > ordering rules here...
> >
> > I also had to get out K&R. The reason I decided to look it up and make
> > the change wast that with the parenthesis I also still needed to think
> > about the precedence. Adding the parenthesis in no way makes the
> > precedence *more* clear. And the checkpatch warning of course.
> >
> > I ran this code to check it
> >
> > #include <stdio.h>
> > #include <stdint.h>
> >
> > /*
> > * Confirm precedence
> > */
> >
> > int main(void)
> > {
> > uint8_t buf[3];
> > uint8_t *bp = buf;
> > uint8_t data;
> >
> > buf[0] = 0;
> > buf[1] = 1;
> > buf[2] = 2;
> >
> > printf("buf = [%d, %d, %d]\n",(int)buf[0], (int)buf[1], (int)buf[2]);
> > printf("bp before: %d\n", (int)*bp);
> >
> > /* data = *(bp)++; */
> > data = *bp++;
> >
> > printf("bp after: %d\n", (int)*bp);
> >
> > return(0);
> > }
>
> Great, so everyone has to check it is valid, so please leave it alone.
> We write code for people to read, and be able to modify in the future.
> Don't force someone to have to really think hard about something trivial
> like this.
Understood. Will drop this change from next version.
Greg please stop reading this email now.
thanks,
Tobin.
For correctness; the dummy code above is not complete. It should be
#include <stdio.h>
#include <stdint.h>
/*
* Confirm statements equivalent
*
* data = *(priv->rxp)++;
* data = *priv->rxp++;
*/
int one = 1;
int two = 2;
struct foo_t {
int *v;
}foos[] = {
{&one},
{&two},
};
int main(void)
{
struct foo_t *fp = foos;
int data = -1;
printf("before: fp->v: %d (%d) data: %d\n", *fp->v, *(fp->v), data);
data = *(fp->v)++;
/* data = *fp->v++; */
printf("after: fp->v: %d (%d) data: %d\n", *fp->v, *(fp->v), data);
return(0);
}
I was wrong *(foo->baz)++ is easier to read than *foo->baz++
thanks,
Tobin.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
