public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
@ 2018-02-21 19:28 Rodrigo Siqueira
  2018-02-21 20:01 ` Julia Lawall
  2018-02-21 21:01 ` Daniel Baluta
  0 siblings, 2 replies; 7+ messages in thread
From: Rodrigo Siqueira @ 2018-02-21 19:28 UTC (permalink / raw)
  To: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler
  Cc: daniel.baluta, linux-iio, kernel-janitors, linux-kernel

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
  2018-02-21 19:28 [PATCH v2] iio:dummy: Replace S_IWUSR by 0200 Rodrigo Siqueira
@ 2018-02-21 20:01 ` Julia Lawall
  2018-02-22  8:11   ` Phil Reid
  2018-02-21 21:01 ` Daniel Baluta
  1 sibling, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2018-02-21 20:01 UTC (permalink / raw)
  To: Rodrigo Siqueira
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, daniel.baluta, linux-iio, kernel-janitors,
	linux-kernel



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
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
  2018-02-21 19:28 [PATCH v2] iio:dummy: Replace S_IWUSR by 0200 Rodrigo Siqueira
  2018-02-21 20:01 ` Julia Lawall
@ 2018-02-21 21:01 ` Daniel Baluta
  2018-02-22  7:43   ` Dan Carpenter
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Baluta @ 2018-02-21 21:01 UTC (permalink / raw)
  To: Rodrigo Siqueira
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Daniel Baluta, linux-iio, kernel-janitors,
	Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
  2018-02-21 21:01 ` Daniel Baluta
@ 2018-02-22  7:43   ` Dan Carpenter
  2018-02-22  7:53     ` Daniel Baluta
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2018-02-22  7:43 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: Rodrigo Siqueira, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Daniel Baluta,
	linux-iio, kernel-janitors, Linux Kernel Mailing List

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
  2018-02-22  7:43   ` Dan Carpenter
@ 2018-02-22  7:53     ` Daniel Baluta
  2018-02-22  8:25       ` Dan Carpenter
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Baluta @ 2018-02-22  7:53 UTC (permalink / raw)
  To: dan.carpenter@oracle.com, daniel.baluta@gmail.com
  Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	lars@metafoo.de, knaack.h@gmx.de, jic23@kernel.org,
	linux-iio@vger.kernel.org, rodrigosiqueiramelo@gmail.com,
	pmeerw@pmeerw.net

SGkgRGFuLA0KDQpPbiBKbywgMjAxOC0wMi0yMiBhdCAxMDo0MyArMDMwMCwgRGFuIENhcnBlbnRl
ciB3cm90ZToNCj4gT24gV2VkLCBGZWIgMjEsIDIwMTggYXQgMTE6MDE6NTBQTSArMDIwMCwgRGFu
aWVsIEJhbHV0YSB3cm90ZToNCj4gPiANCj4gPiBPbiBXZWQsIEZlYiAyMSwgMjAxOCBhdCA5OjI4
IFBNLCBSb2RyaWdvIFNpcXVlaXJhDQo+ID4gPHJvZHJpZ29zaXF1ZWlyYW1lbG9AZ21haWwuY29t
PiB3cm90ZToNCj4gPiA+IA0KPiA+ID4gVGhpcyBwYXRjaCBmaXhlcyB0aGUgY2hlY2twYXRjaC5w
bCB3YXJuaW5nOg0KPiA+ID4gDQo+ID4gPiBkcml2ZXJzL2lpby9kdW1teS9paW9fZHVtbXlfZXZn
ZW4uYzoxNTE6IFdBUk5JTkc6IFN5bWJvbGljIHBlcm1pc3Npb25zDQo+ID4gPiAnU19JV1VTUicg
YXJlIG5vdCBwcmVmZXJyZWQuIENvbnNpZGVyIHVzaW5nIG9jdGFsIHBlcm1pc3Npb25zICcwMjAw
Jy4NCj4gPiA+IA0KPiA+ID4gLi4uIFdoeSB0aGlzICIuLi4iIDopPw0KPiA+IENvbW1pdCBzdWJq
ZWN0IGNvdWxkIGJlIGRvbmUgYmV0dGVyLiBJdCBpcyBwcmV0dHkgb2J2aW91cyBmcm9tIHRoZSBj
b2RlIHRoYXQNCj4gPiB3ZSBjaGFuZ2UgU19JV1VTUiB3aXRoIDAyMDAuDQo+ID4gDQo+ID4gQmV0
dGVyIG1lc3NhZ2U6DQo+ID4gDQo+ID4gaWlvOmR1bW15OiBGaXggcG9rZV9ldk4gZmlsZSBwZXJt
aXNzaW9ucw0KPiBQbGVhc2Ugc3RvcCB0ZWxsaW5nIHBlb3BsZSB0byBzYXkgIkZpeCIgd2hlbiBp
dCdzIG5vdCBhIGJ1ZyBmaXguLi4NCj4gDQoNCkkgYWdyZWUgd2l0aCB5b3UgdGhhdCBoZXJlICJG
aXgiIGlzIGFuIG92ZXJzdGF0ZW1lbnQuDQoNCj4gQWxzbyB3aG8gY2FyZXM/wqDCoFRoZSBjb21t
aXQgbWVzc2FnZSBpcyBwZXJmZWN0bHkgY2xlYXIuDQoNCkkgZG8gY2FyZSBhYm91dCBuZXdjb21l
cnMgcmVhbGx5IGxlYXJuaW5nIG9uIGhvdyB0byB3cml0ZSBhIHByb3Blcg0KY29tbWl0IG1lc3Nh
Z2UuDQoNClRoZSBjb21taXQgbWVzc3NhZ2Ugc2hvdWxkIHJlYWxseSBzYXkgd2h5IHRoZSBwYXRj
aCBpcyBuZWVkZWQsIG5vIHdoYXQgdGhlIHBhdGNoIGRvZXMuDQoNCkFmdGVyIHNldmVyYWwgdHJp
dmlhbCBwYXRjaGVzIG5ld2NvbWVycyB3aWxsIGdldCBpbnRvIG1vcmUgc2VyaW91cyBzdHVmZiBh
bmQgSSB3b3VsZG4ndA0Kd2FudCB0byBzZWUgYSBwYXRjaCB3aXRoIGNvbW1pdCBtZXNzYWdlIGxp
a2UgdGhpczoNCg0KaWlvOiBDaGFuZ2UgYml0IDEgb2Ygc3RhdHVzIHJlZ2lzdGVyDQoNCmJ1dCBv
bmUgdGhhdCBsb29rcyBsaWtlIHRoaXM6DQoNCmlpbzogU2V0IHBvd2VyIHVwIG9uIGNoaXAgcHJv
YmUNCg0KdGhhbmtzLA0KRGFuaWVsLg0KDQoNCg=

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
  2018-02-21 20:01 ` Julia Lawall
@ 2018-02-22  8:11   ` Phil Reid
  0 siblings, 0 replies; 7+ messages in thread
From: Phil Reid @ 2018-02-22  8:11 UTC (permalink / raw)
  To: Julia Lawall, Rodrigo Siqueira
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, daniel.baluta, linux-iio, kernel-janitors,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
  2018-02-22  7:53     ` Daniel Baluta
@ 2018-02-22  8:25       ` Dan Carpenter
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2018-02-22  8:25 UTC (permalink / raw)
  To: Daniel Baluta
  Cc: daniel.baluta@gmail.com, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org, lars@metafoo.de, knaack.h@gmx.de,
	jic23@kernel.org, linux-iio@vger.kernel.org,
	rodrigosiqueiramelo@gmail.com, pmeerw@pmeerw.net

> > 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 kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-02-22  8:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-21 19:28 [PATCH v2] iio:dummy: Replace S_IWUSR by 0200 Rodrigo Siqueira
2018-02-21 20:01 ` Julia Lawall
2018-02-22  8:11   ` Phil Reid
2018-02-21 21:01 ` Daniel Baluta
2018-02-22  7:43   ` Dan Carpenter
2018-02-22  7:53     ` Daniel Baluta
2018-02-22  8:25       ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox