| Message ID | 20180221192807.qfyhp7z27f6r6p47@smtp.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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.
> > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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(-)