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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
>
> 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 <[email protected]>
> > ---
> > 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
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
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
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 <[email protected]>
> ---
> 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 [email protected].
> To post to this group, send email to [email protected].
> 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.
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 <[email protected]>
>>> ---
>>> 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
>>>
>>
>