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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira
<[email protected]> 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
On Wed, Feb 21, 2018 at 11:01:50PM +0200, Daniel Baluta wrote:
> On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira
> <[email protected]> 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
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
> > <[email protected]> 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.
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
> > 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