| Message ID | 1601385283-26144-11-git-send-email-Julia.Lawall@inria.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: use semicolons rather than commas to separate statements | expand |
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! CJ
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. > CJ
On Tue, 29 Sep 2020, 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! Oops, yes, I meant to make a comment about it. Joe may have sent this patch already. At least he has sent a patch on an issue like this. julia
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. No, I didn't do it by hand. Did you already send this one? Maybe I should resend it with a more informative log message. julia > > CJ >
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. Joe already submitted this patch so please ignore my version: https://lkml.org/lkml/2020/8/23/178 julia
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
On Tue, 2020-09-29 at 18:44 +0200, Julia Lawall wrote: > 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. > > No, I didn't do it by hand. Did you already send this one? Maybe I > should resend it with a more informative log message. I did not send a patch for this one. Yes, I think you should say you are fixing a likely defect here.
On Tue, 2020-09-29 at 15:21 -0700, Joe Perches wrote:
> I did not send a patch for this one.
Well, I guess I did and forgot.
I thought this was about the braces and semicolons
addition around multi-statement commas, and I
didn't send a patch for this file for that.
cheers, Joe
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");
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(-)