[PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Rodrigo Siqueira]

This patch fixes the checkpatch.pl warning:

drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions
'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
...

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
Changes in v2:
  - Make the commit message clearer.
  - Fix just a single part of the code.

 drivers/iio/dummy/iio_dummy_evgen.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c
index efd0005f59b4..16ea547f79f0 100644
--- a/drivers/iio/dummy/iio_dummy_evgen.c
+++ b/drivers/iio/dummy/iio_dummy_evgen.c
@@ -148,16 +148,16 @@ static ssize_t iio_evgen_poke(struct device *dev,
 	return len;
 }
 
-static IIO_DEVICE_ATTR(poke_ev0, S_IWUSR, NULL, &iio_evgen_poke, 0);
-static IIO_DEVICE_ATTR(poke_ev1, S_IWUSR, NULL, &iio_evgen_poke, 1);
-static IIO_DEVICE_ATTR(poke_ev2, S_IWUSR, NULL, &iio_evgen_poke, 2);
-static IIO_DEVICE_ATTR(poke_ev3, S_IWUSR, NULL, &iio_evgen_poke, 3);
-static IIO_DEVICE_ATTR(poke_ev4, S_IWUSR, NULL, &iio_evgen_poke, 4);
-static IIO_DEVICE_ATTR(poke_ev5, S_IWUSR, NULL, &iio_evgen_poke, 5);
-static IIO_DEVICE_ATTR(poke_ev6, S_IWUSR, NULL, &iio_evgen_poke, 6);
-static IIO_DEVICE_ATTR(poke_ev7, S_IWUSR, NULL, &iio_evgen_poke, 7);
-static IIO_DEVICE_ATTR(poke_ev8, S_IWUSR, NULL, &iio_evgen_poke, 8);
-static IIO_DEVICE_ATTR(poke_ev9, S_IWUSR, NULL, &iio_evgen_poke, 9);
+static IIO_DEVICE_ATTR(poke_ev0, 0200, NULL, &iio_evgen_poke, 0);
+static IIO_DEVICE_ATTR(poke_ev1, 0200, NULL, &iio_evgen_poke, 1);
+static IIO_DEVICE_ATTR(poke_ev2, 0200, NULL, &iio_evgen_poke, 2);
+static IIO_DEVICE_ATTR(poke_ev3, 0200, NULL, &iio_evgen_poke, 3);
+static IIO_DEVICE_ATTR(poke_ev4, 0200, NULL, &iio_evgen_poke, 4);
+static IIO_DEVICE_ATTR(poke_ev5, 0200, NULL, &iio_evgen_poke, 5);
+static IIO_DEVICE_ATTR(poke_ev6, 0200, NULL, &iio_evgen_poke, 6);
+static IIO_DEVICE_ATTR(poke_ev7, 0200, NULL, &iio_evgen_poke, 7);
+static IIO_DEVICE_ATTR(poke_ev8, 0200, NULL, &iio_evgen_poke, 8);
+static IIO_DEVICE_ATTR(poke_ev9, 0200, NULL, &iio_evgen_poke, 9);
 
 static struct attribute *iio_evgen_attrs[] = {
 	&iio_dev_attr_poke_ev0.dev_attr.attr,
-- 
2.16.2

Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Julia Lawall]

On Wed, 21 Feb 2018, Rodrigo Siqueira wrote:

> This patch fixes the checkpatch.pl warning:
>
> drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions
> 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.

I haven't studied up on it in great detail, but isn't there a more
specific macro that doesn't need a permission argument at all?

julia

> ...
>
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> ---
> Changes in v2:
>   - Make the commit message clearer.
>   - Fix just a single part of the code.
>
>  drivers/iio/dummy/iio_dummy_evgen.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/dummy/iio_dummy_evgen.c b/drivers/iio/dummy/iio_dummy_evgen.c
> index efd0005f59b4..16ea547f79f0 100644
> --- a/drivers/iio/dummy/iio_dummy_evgen.c
> +++ b/drivers/iio/dummy/iio_dummy_evgen.c
> @@ -148,16 +148,16 @@ static ssize_t iio_evgen_poke(struct device *dev,
>  	return len;
>  }
>
> -static IIO_DEVICE_ATTR(poke_ev0, S_IWUSR, NULL, &iio_evgen_poke, 0);
> -static IIO_DEVICE_ATTR(poke_ev1, S_IWUSR, NULL, &iio_evgen_poke, 1);
> -static IIO_DEVICE_ATTR(poke_ev2, S_IWUSR, NULL, &iio_evgen_poke, 2);
> -static IIO_DEVICE_ATTR(poke_ev3, S_IWUSR, NULL, &iio_evgen_poke, 3);
> -static IIO_DEVICE_ATTR(poke_ev4, S_IWUSR, NULL, &iio_evgen_poke, 4);
> -static IIO_DEVICE_ATTR(poke_ev5, S_IWUSR, NULL, &iio_evgen_poke, 5);
> -static IIO_DEVICE_ATTR(poke_ev6, S_IWUSR, NULL, &iio_evgen_poke, 6);
> -static IIO_DEVICE_ATTR(poke_ev7, S_IWUSR, NULL, &iio_evgen_poke, 7);
> -static IIO_DEVICE_ATTR(poke_ev8, S_IWUSR, NULL, &iio_evgen_poke, 8);
> -static IIO_DEVICE_ATTR(poke_ev9, S_IWUSR, NULL, &iio_evgen_poke, 9);
> +static IIO_DEVICE_ATTR(poke_ev0, 0200, NULL, &iio_evgen_poke, 0);
> +static IIO_DEVICE_ATTR(poke_ev1, 0200, NULL, &iio_evgen_poke, 1);
> +static IIO_DEVICE_ATTR(poke_ev2, 0200, NULL, &iio_evgen_poke, 2);
> +static IIO_DEVICE_ATTR(poke_ev3, 0200, NULL, &iio_evgen_poke, 3);
> +static IIO_DEVICE_ATTR(poke_ev4, 0200, NULL, &iio_evgen_poke, 4);
> +static IIO_DEVICE_ATTR(poke_ev5, 0200, NULL, &iio_evgen_poke, 5);
> +static IIO_DEVICE_ATTR(poke_ev6, 0200, NULL, &iio_evgen_poke, 6);
> +static IIO_DEVICE_ATTR(poke_ev7, 0200, NULL, &iio_evgen_poke, 7);
> +static IIO_DEVICE_ATTR(poke_ev8, 0200, NULL, &iio_evgen_poke, 8);
> +static IIO_DEVICE_ATTR(poke_ev9, 0200, NULL, &iio_evgen_poke, 9);
>
>  static struct attribute *iio_evgen_attrs[] = {
>  	&iio_dev_attr_poke_ev0.dev_attr.attr,
> --
> 2.16.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Daniel Baluta]

On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
> This patch fixes the checkpatch.pl warning:
>
> drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions
> 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.

> ... Why this "..." :)?

Commit subject could be done better. It is pretty obvious from the code that
we change S_IWUSR with 0200.

Better message:

iio:dummy: Fix poke_evN file permissions

Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Dan Carpenter]

On Wed, Feb 21, 2018 at 11:01:50PM +0200, Daniel Baluta wrote:
> On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> > This patch fixes the checkpatch.pl warning:
> >
> > drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions
> > 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
> 
> > ... Why this "..." :)?
> 
> Commit subject could be done better. It is pretty obvious from the code that
> we change S_IWUSR with 0200.
> 
> Better message:
> 
> iio:dummy: Fix poke_evN file permissions

Please stop telling people to say "Fix" when it's not a bug fix...

Also who cares?  The commit message is perfectly clear.

regards,
dan carpenter

Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Daniel Baluta]

Hi Dan,

On Jo, 2018-02-22 at 10:43 +0300, Dan Carpenter wrote:
> On Wed, Feb 21, 2018 at 11:01:50PM +0200, Daniel Baluta wrote:
> > 
> > On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira
> > <rodrigosiqueiramelo@gmail.com> wrote:
> > > 
> > > This patch fixes the checkpatch.pl warning:
> > > 
> > > drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions
> > > 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
> > > 
> > > ... Why this "..." :)?
> > Commit subject could be done better. It is pretty obvious from the code that
> > we change S_IWUSR with 0200.
> > 
> > Better message:
> > 
> > iio:dummy: Fix poke_evN file permissions
> Please stop telling people to say "Fix" when it's not a bug fix...
> 

I agree with you that here "Fix" is an overstatement.

> Also who cares?  The commit message is perfectly clear.

I do care about newcomers really learning on how to write a proper
commit message.

The commit messsage should really say why the patch is needed, no what the patch does.

After several trivial patches newcomers will get into more serious stuff and I wouldn't
want to see a patch with commit message like this:

iio: Change bit 1 of status register

but one that looks like this:

iio: Set power up on chip probe

thanks,
Daniel.

Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Phil Reid]

On 22/02/2018 04:01, Julia Lawall wrote:
> 
> On Wed, 21 Feb 2018, Rodrigo Siqueira wrote:
> 
>> This patch fixes the checkpatch.pl warning:
>>
>> drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions
>> 'S_IWUSR' are not preferred. Consider using octal permissions '0200'.
> I haven't studied up on it in great detail, but isn't there a more
> specific macro that doesn't need a permission argument at all?
> 

Probably thinking of IIO_DEVICE_ATTR_RO / IIO_DEVICE_ATTR_WO
But they don't provide flexibility for the show / store method.

-- 
Regards
Phil Reid

Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200

[Dan Carpenter]

> > Also who cares?  The commit message is perfectly clear.
> 
> I do care about newcomers really learning on how to write a proper
> commit message.
> 
> The commit messsage should really say why the patch is needed, no what the patch does.
> 

It fixes a checkpatch warning.  The warning was right there in the patch
description!

> After several trivial patches newcomers will get into more serious stuff and I wouldn't
> want to see a patch with commit message like this:
> 
> iio: Change bit 1 of status register
> 

You seem to be complaining about about an imaginary patch from the
future???  If you've really built a time machine then focus on killing
Hitler instead of complaining about trivial bullshit.

regards,
dan carpenter