Re: [PATCH 10/20] media: lmedm04: use semicolons rather than commas to separate statements

From: Julia Lawall <julia.lawall@inria.fr>
To: Joe Perches <joe@perches.com>
Cc: "Christophe JAILLET" <christophe.jaillet@wanadoo.fr>,
	"Malcolm Priestley" <tvboxspy@gmail.com>,
	"Valdis Klētnieks" <valdis.kletnieks@vt.edu>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	kernel-janitors@vger.kernel.org,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 10/20] media: lmedm04: use semicolons rather than commas to separate statements
Date: Tue, 29 Sep 2020 20:43:18 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2009292041580.2808@hadrien> (raw)
In-Reply-To: <c79b36c12c978d38f3ad89b1c659871a@perches.com>

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

On Tue, 29 Sep 2020, Joe Perches wrote:

> On 2020-09-29 09:00, Christophe JAILLET wrote:
> > Le 29/09/2020 à 15:14, Julia Lawall a écrit :
> > > Replace commas with semicolons.  Commas introduce unnecessary
> > > variability in the code structure and are hard to see.  What is done
> > > is essentially described by the following Coccinelle semantic patch
> > > (http://coccinelle.lip6.fr/):
> > >
> > > // <smpl>
> > > @@ expression e1,e2; @@
> > > e1
> > > -,
> > > +;
> > > e2
> > > ... when any
> > > // </smpl>
> > >
> > > Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>
> > >
> > > ---
> > >   drivers/media/usb/dvb-usb-v2/lmedm04.c |    2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> > > b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> > > index 5a7a9522d46d..9ddda8d68ee0 100644
> > > --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> > > +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> > > @@ -391,7 +391,7 @@ static int lme2510_int_read(struct dvb_usb_adapter
> > > *adap)
> > >   	ep = usb_pipe_endpoint(d->udev, lme_int->lme_urb->pipe);
> > >     	if (usb_endpoint_type(&ep->desc) == USB_ENDPOINT_XFER_BULK)
> > > -		lme_int->lme_urb->pipe = usb_rcvbulkpipe(d->udev, 0xa),
> > > +		lme_int->lme_urb->pipe = usb_rcvbulkpipe(d->udev, 0xa);
> > >     	usb_submit_urb(lme_int->lme_urb, GFP_ATOMIC);
> > >   	info("INT Interrupt Service Started");
> > >
> > >
> > Ouch!
> >
> > This one looks like a real issue!
>
>
> Julia?  Did you do this one by hand?  This actually changes logic which I did
> not expectthe cocci script to do.

I think that there is something wrong with the implementation of the rule
that considers the case where the two statements are not on the same line.
For some reason, it is not triggering the addition of {}.  I will have to
look into it.  Otherwise, all of the {}s were put there automatically.

julia