Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1613035 > unrolled thread

[PATCH] iio: adc: replace comma with a semicolon

Back to article view | Back to linux.kernel

Contents

  [PATCH] iio: adc: replace comma with a semicolon Arushi Singhal <arushisinghal19971997@gmail.com> - 2017-03-30 14:50 +0200
    Re: [PATCH] iio: adc: replace comma with a semicolon Peter Meerwald-Stadler <pmeerw@pmeerw.net> - 2017-03-30 15:40 +0200
      Re: [PATCH] iio: adc: replace comma with a semicolon Jonathan Cameron <jic23@kernel.org> - 2017-04-01 12:10 +0200
    Re: [PATCH] iio: adc: replace comma with a semicolon jacopo <jacopo@jmondi.org> - 2017-03-30 15:40 +0200
      Re: [PATCH] iio: adc: replace comma with a semicolon Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-03-30 19:30 +0200
        Re: [PATCH] iio: adc: replace comma with a semicolon jacopo <jacopo@jmondi.org> - 2017-03-30 20:10 +0200
    Re: [Outreachy kernel] [PATCH] iio: adc: replace comma with a  semicolon Alison Schofield <amsfield22@gmail.com> - 2017-03-30 20:30 +0200

#1613035 — [PATCH] iio: adc: replace comma with a semicolon

Replace a comma between expression statements by a semicolon. This
changes the semantics of the code, but given the current indentation
appears to be what is intended.
A simplified version of the Coccinelle semantic patch that performs this
transformation is as follows:

// <smpl>
@r@
expression e1,e2;
@@

 e1
-,
+;
 e2;
// </smpl>

Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
---
 drivers/iio/adc/max11100.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
index 23c060e1b663..1180bcc22ff1 100644
--- a/drivers/iio/adc/max11100.c
+++ b/drivers/iio/adc/max11100.c
@@ -124,8 +124,8 @@ static int max11100_probe(struct spi_device *spi)
 	indio_dev->name = "max11100";
 	indio_dev->info = &max11100_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = max11100_channels,
-	indio_dev->num_channels = ARRAY_SIZE(max11100_channels),
+	indio_dev->channels = max11100_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max11100_channels);
 
 	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
 	if (IS_ERR(state->vref_reg))
-- 
2.11.0

[toc] | [next] | [standalone]

#1613073

> On Thu, Mar 30, 2017 at 06:16:03PM +0530, Arushi Singhal wrote:
> > Replace a comma between expression statements by a semicolon. This
> > changes the semantics of the code, but given the current indentation
> > appears to be what is intended.

> You can simply say that this fixes what appears to be a bug to me.
> I wonder how this does even compile..

it's valid C and I think it does the correct thing (i.e. assign those 
variables); the style is weird obviously

> [1] https://marc.info/?l=linux-iio&m=148475728729617&w=2
> 
> > Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> > ---
> >  drivers/iio/adc/max11100.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> > index 23c060e1b663..1180bcc22ff1 100644
> > --- a/drivers/iio/adc/max11100.c
> > +++ b/drivers/iio/adc/max11100.c
> > @@ -124,8 +124,8 @@ static int max11100_probe(struct spi_device *spi)
> >  	indio_dev->name = "max11100";
> >  	indio_dev->info = &max11100_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > -	indio_dev->channels = max11100_channels,
> > -	indio_dev->num_channels = ARRAY_SIZE(max11100_channels),
> > +	indio_dev->channels = max11100_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(max11100_channels);
> >  
> >  	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
> >  	if (IS_ERR(state->vref_reg))
> > -- 
> > 2.11.0
> > 
> 

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

[toc] | [prev] | [next] | [standalone]

#1614433

On 30/03/17 14:38, Peter Meerwald-Stadler wrote:
> 
>> On Thu, Mar 30, 2017 at 06:16:03PM +0530, Arushi Singhal wrote:
>>> Replace a comma between expression statements by a semicolon. This
>>> changes the semantics of the code, but given the current indentation
>>> appears to be what is intended.
> 
>> You can simply say that this fixes what appears to be a bug to me.
>> I wonder how this does even compile..
> 
> it's valid C and I think it does the correct thing (i.e. assign those 
> variables); the style is weird obviously
Hohum I am clearly going crazy in my old age.
Not a bug, but nice to fix so
Applied to the togreg branch of iio.git.  Sorry for messing my little
change up Jacopo!  Always worth checking a maintainer isn't being
an idiot when they apply your patches. As others will certify we
make plenty of mistakes.

Jonathan

> 
>> [1] https://marc.info/?l=linux-iio&m=148475728729617&w=2
>>
>>> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
>>> ---
>>>  drivers/iio/adc/max11100.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
>>> index 23c060e1b663..1180bcc22ff1 100644
>>> --- a/drivers/iio/adc/max11100.c
>>> +++ b/drivers/iio/adc/max11100.c
>>> @@ -124,8 +124,8 @@ static int max11100_probe(struct spi_device *spi)
>>>  	indio_dev->name = "max11100";
>>>  	indio_dev->info = &max11100_info;
>>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>> -	indio_dev->channels = max11100_channels,
>>> -	indio_dev->num_channels = ARRAY_SIZE(max11100_channels),
>>> +	indio_dev->channels = max11100_channels;
>>> +	indio_dev->num_channels = ARRAY_SIZE(max11100_channels);
>>>  
>>>  	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
>>>  	if (IS_ERR(state->vref_reg))
>>> -- 
>>> 2.11.0
>>>
>>
> 

[toc] | [prev] | [next] | [standalone]

#1613076

Hi Arushi,
   thanks for your patch

On Thu, Mar 30, 2017 at 06:16:03PM +0530, Arushi Singhal wrote:
> Replace a comma between expression statements by a semicolon. This
> changes the semantics of the code, but given the current indentation
> appears to be what is intended.
> A simplified version of the Coccinelle semantic patch that performs this
> transformation is as follows:
> 
> // <smpl>
> @r@
> expression e1,e2;
> @@
> 
>  e1
> -,
> +;
>  e2;
> // </smpl>
> 

You can simply say that this fixes what appears to be a bug to me.
I wonder how this does even compile..

Jonathan, since I assume you picked this up and changed those 2 lines
(original patch here [1]), was this intentional?

Thanks
   j

[1] https://marc.info/?l=linux-iio&m=148475728729617&w=2

> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
>  drivers/iio/adc/max11100.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> index 23c060e1b663..1180bcc22ff1 100644
> --- a/drivers/iio/adc/max11100.c
> +++ b/drivers/iio/adc/max11100.c
> @@ -124,8 +124,8 @@ static int max11100_probe(struct spi_device *spi)
>  	indio_dev->name = "max11100";
>  	indio_dev->info = &max11100_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = max11100_channels,
> -	indio_dev->num_channels = ARRAY_SIZE(max11100_channels),
> +	indio_dev->channels = max11100_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max11100_channels);
>  
>  	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(state->vref_reg))
> -- 
> 2.11.0
> 

[toc] | [prev] | [next] | [standalone]

#1613310

On 30/03/2017 at 15:30:46 +0200, jacopo wrote:
> Hi Arushi,
>    thanks for your patch
> 
> On Thu, Mar 30, 2017 at 06:16:03PM +0530, Arushi Singhal wrote:
> > Replace a comma between expression statements by a semicolon. This
> > changes the semantics of the code, but given the current indentation
> > appears to be what is intended.
> > A simplified version of the Coccinelle semantic patch that performs this
> > transformation is as follows:
> > 
> > // <smpl>
> > @r@
> > expression e1,e2;
> > @@
> > 
> >  e1
> > -,
> > +;
> >  e2;
> > // </smpl>
> > 
> 
> You can simply say that this fixes what appears to be a bug to me.
> I wonder how this does even compile..
> 

It is not a bug, it is perfectly valid and working as expected/intended.
This is just a cosmetic change.

See https://en.wikipedia.org/wiki/Comma_operator if you want to
understand.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1613338

Hi Alexandre
   hope you're doing good!

On Thu, Mar 30, 2017 at 07:23:12PM +0200, Alexandre Belloni wrote:
> On 30/03/2017 at 15:30:46 +0200, jacopo wrote:
> > Hi Arushi,
> >    thanks for your patch
> > 
> > On Thu, Mar 30, 2017 at 06:16:03PM +0530, Arushi Singhal wrote:
> > > Replace a comma between expression statements by a semicolon. This
> > > changes the semantics of the code, but given the current indentation
> > > appears to be what is intended.
> > > A simplified version of the Coccinelle semantic patch that performs this
> > > transformation is as follows:
> > > 
> > > // <smpl>
> > > @r@
> > > expression e1,e2;
> > > @@
> > > 
> > >  e1
> > > -,
> > > +;
> > >  e2;
> > > // </smpl>
> > > 
> > 
> > You can simply say that this fixes what appears to be a bug to me.
> > I wonder how this does even compile..
> > 
> 
> It is not a bug, it is perfectly valid and working as expected/intended.
> This is just a cosmetic change.
> 
> See https://en.wikipedia.org/wiki/Comma_operator if you want to
> understand.
> 

Thanks! That's valid and legal (I should have thought at multiple
expression in for(;;) as an example).
Ignore my comment then (I still don't think it was "intended" as it's
quite unusual to see commas there :)

Thanks
   j

> 
> -- 
> Alexandre Belloni, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1613352 — Re: [Outreachy kernel] [PATCH] iio: adc: replace comma with a semicolon

On Thu, Mar 30, 2017 at 06:16:03PM +0530, Arushi Singhal wrote:
> Replace a comma between expression statements by a semicolon. This
> changes the semantics of the code, but given the current indentation
> appears to be what is intended.

Hi Arushi,

Well, you've gotten a lot of comments on this one, and I too, couldn't
resist ;)  My comment is to test things like this out. Of course, you're
not going to go off and test the max11100 driver (BTW - driver name in
Subject line please), but you can write a simple C program to verify
the behavior of commas vs semi-colons in assignment statements.
That'll prove it to yourself, and then you can write the changelog
with confidence. (That this is cosmetic ;))

It's a useful habit to pick up.  When you see a code segment that
makes you wonder, you can often pluck it out of context and try
it in a simple c program.

alisons

> A simplified version of the Coccinelle semantic patch that performs this
> transformation is as follows:
> 
> // <smpl>
> @r@
> expression e1,e2;
> @@
> 
>  e1
> -,
> +;
>  e2;
> // </smpl>
> 
> Signed-off-by: Arushi Singhal <arushisinghal19971997@gmail.com>
> ---
>  drivers/iio/adc/max11100.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
> index 23c060e1b663..1180bcc22ff1 100644
> --- a/drivers/iio/adc/max11100.c
> +++ b/drivers/iio/adc/max11100.c
> @@ -124,8 +124,8 @@ static int max11100_probe(struct spi_device *spi)
>  	indio_dev->name = "max11100";
>  	indio_dev->info = &max11100_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = max11100_channels,
> -	indio_dev->num_channels = ARRAY_SIZE(max11100_channels),
> +	indio_dev->channels = max11100_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max11100_channels);
>  
>  	state->vref_reg = devm_regulator_get(&spi->dev, "vref");
>  	if (IS_ERR(state->vref_reg))
> -- 
> 2.11.0
> 
> -- 
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170330124603.GA29301%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web