staging: speakup: speakup_acntpc.c Consider octal permissions
PATCH 1/1 staging: speakup: speakup_acntpc.c Consider octal permissions
Make suggested checkpatch modifications for
WARNING: Symbolic permissions 'S_IWUSR | S_IRUGO' are not preferred. Consider using octal permissions '0644'.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.
---
I am new to making trivial patches and do not make some for a few type of warnings.
This is one of them as I am not fully certain that it is as easy as this.
The replacing of 'S_IWUSR | S_IRUGO' with '0644' seems simple enough.
However the adding of '_RW' to '__ATTR' to make '__ATTR_RW' I saw in a reply to a patch and am not sure that it would be relevant in this case.
I also made a previous patch adding spaces around '|' and want to know if just replacing 'S_IWUSR|S_IRUGO' with '0644' in one shot would be acceptable since my being new and not fixing just one type of warning per patch. Seems straight forward but I have spammed other peoples email and the mailing list enough with improper patches.
drivers/staging/speakup/speakup_acntpc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
drivers/staging/speakup/speakup_acntpc.c +13 -13
@@ -56,28 +56,28 @@ static struct var_t vars[] = {/* These attributes will appear in /sys/accessibility/speakup/acntpc. */ static struct kobj_attribute caps_start_attribute = -__ATTR (caps_start,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_start,0644 , spk_var_show, spk_var_store);static struct kobj_attribute caps_stop_attribute = -__ATTR (caps_stop,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_stop,0644 , spk_var_show, spk_var_store);static struct kobj_attribute pitch_attribute = -__ATTR (pitch,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (pitch,0644 , spk_var_show, spk_var_store);static struct kobj_attribute rate_attribute = -__ATTR (rate,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (rate,0644 , spk_var_show, spk_var_store);static struct kobj_attribute tone_attribute = -__ATTR (tone,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (tone,0644 , spk_var_show, spk_var_store);static struct kobj_attribute vol_attribute = -__ATTR (vol,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (vol,0644 , spk_var_show, spk_var_store);static struct kobj_attribute delay_time_attribute = -__ATTR (delay_time,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (delay_time,0644 , spk_var_show, spk_var_store);static struct kobj_attribute direct_attribute = -__ATTR (direct,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (direct,0644 , spk_var_show, spk_var_store);static struct kobj_attribute full_time_attribute = -__ATTR (full_time,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (full_time,0644 , spk_var_show, spk_var_store);static struct kobj_attribute jiffy_delta_attribute = -__ATTR (jiffy_delta,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (jiffy_delta,0644 , spk_var_show, spk_var_store);static struct kobj_attribute trigger_time_attribute = -__ATTR (trigger_time,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (trigger_time,0644 , spk_var_show, spk_var_store);/* * Create a group of attributes so that we can create and destroy them all
@@ -306,8 +306,8 @@ static void accent_release(void) speakup_info.port_tts =0 ; } -module_param_named (port, port_forced, int,S_IRUGO ); -module_param_named (start, synth_acntpc.startup, short,S_IRUGO ); +module_param_named (port, port_forced, int,0444 ); +module_param_named (start, synth_acntpc.startup, short,0444 );MODULE_PARM_DESC (port,"Set the port for the synthesizer (override probing)." );MODULE_PARM_DESC (start,"Start the synthesizer once it is loaded." );
Signature · 1 line
2.1.4
Greg KH
maintainerPATCH 1/1 staging: speakup: speakup_acntpc.c Consider octal permissions
Quoted 32 lines · Walt Feasel
Make suggested checkpatch modifications for
WARNING: Symbolic permissions 'S_IWUSR|S_IRUGO' are not preferred. Consider using octal permissions '0644'.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.Signed-off-by Walt Feasel <[email protected]>---
I am new to making trivial patches and do not make some for a few type of warnings.
This is one of them as I am not fully certain that it is as easy as this.
The replacing of 'S_IWUSR|S_IRUGO' with '0644' seems simple enough.
However the adding of '_RW' to '__ATTR' to make '__ATTR_RW' I saw in a reply to a patch and am not sure that it would be relevant in this case.
I also made a previous patch adding spaces around '|' and want to know if just replacing 'S_IWUSR|S_IRUGO' with '0644' in one shot would be acceptable since my being new and not fixing just one type of warning per patch. Seems straight forward but I have spammed other peoples email and the mailing list enough with improper patches.drivers/staging/speakup/speakup_acntpc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)drivers/staging/speakup/speakup_acntpc.c +0
-0
drivers/staging/speakup/speakup_acntpc.c static struct var_t vars[] = { · line 59
Quoted 5 lines
drivers/staging/speakup/speakup_acntpc.c +1
-1@@ -56,28 +56,28 @@ static struct var_t vars[] = {/* These attributes will appear in /sys/accessibility/speakup/acntpc. */ static struct kobj_attribute caps_start_attribute = -__ATTR (caps_start,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_start,0644 , spk_var_show, spk_var_store);
This breaks the build :(
PATCH 1/1 staging: speakup: speakup_acntpc.c Consider octal permissions
Quoted 35 lines · Greg KH
Quoted 34 lines · Walt Feasel
Make suggested checkpatch modifications for
WARNING: Symbolic permissions 'S_IWUSR|S_IRUGO' are not preferred. Consider using octal permissions '0644'.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.Signed-off-by Walt Feasel <[email protected]>---
I am new to making trivial patches and do not make some for a few type of warnings.
This is one of them as I am not fully certain that it is as easy as this.
The replacing of 'S_IWUSR|S_IRUGO' with '0644' seems simple enough.
However the adding of '_RW' to '__ATTR' to make '__ATTR_RW' I saw in a reply to a patch and am not sure that it would be relevant in this case.
I also made a previous patch adding spaces around '|' and want to know if just replacing 'S_IWUSR|S_IRUGO' with '0644' in one shot would be acceptable since my being new and not fixing just one type of warning per patch. Seems straight forward but I have spammed other peoples email and the mailing list enough with improper patches.drivers/staging/speakup/speakup_acntpc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)drivers/staging/speakup/speakup_acntpc.c +0
-0@@ -56,28 +56,28 @@ static struct var_t vars[] = {/* These attributes will appear in /sys/accessibility/speakup/acntpc. */
Quoted 6 lines
drivers/staging/speakup/speakup_acntpc.c static struct var_t vars[] = { · line 59
Quoted 3 lines
drivers/staging/speakup/speakup_acntpc.c +1
-1static struct kobj_attribute caps_start_attribute = -__ATTR (caps_start,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_start,0644 , spk_var_show, spk_var_store);This breaks the build :(
I would assume from adding _RW as is didnt seem to be needed but only a visual reference to the octal's permissions without doing the math. I will look deeper into where __ATTR is probally being called then. I'm not really worried with builds yet till I have a better understanging of the issue as I would expect all of them to fail at this point.
Greg KH
maintainerPATCH 1/1 staging: speakup: speakup_acntpc.c Consider octal permissions
Quoted 42 lines · Walt Feasel
Quoted 35 lines · Greg KH
Quoted 34 lines · Walt Feasel
Make suggested checkpatch modifications for
WARNING: Symbolic permissions 'S_IWUSR|S_IRUGO' are not preferred. Consider using octal permissions '0644'.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.Signed-off-by Walt Feasel <[email protected]>---
I am new to making trivial patches and do not make some for a few type of warnings.
This is one of them as I am not fully certain that it is as easy as this.
The replacing of 'S_IWUSR|S_IRUGO' with '0644' seems simple enough.
However the adding of '_RW' to '__ATTR' to make '__ATTR_RW' I saw in a reply to a patch and am not sure that it would be relevant in this case.
I also made a previous patch adding spaces around '|' and want to know if just replacing 'S_IWUSR|S_IRUGO' with '0644' in one shot would be acceptable since my being new and not fixing just one type of warning per patch. Seems straight forward but I have spammed other peoples email and the mailing list enough with improper patches.drivers/staging/speakup/speakup_acntpc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)drivers/staging/speakup/speakup_acntpc.c +0
-0@@ -56,28 +56,28 @@ static struct var_t vars[] = {/* These attributes will appear in /sys/accessibility/speakup/acntpc. */ Quoted 6 lines
drivers/staging/speakup/speakup_acntpc.c static struct var_t vars[] = { · line 59
Quoted 3 lines
drivers/staging/speakup/speakup_acntpc.c +1
-1static struct kobj_attribute caps_start_attribute = -__ATTR (caps_start,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_start,0644 , spk_var_show, spk_var_store);This breaks the build :(
Quoted 6 lines
I would assume from adding
_RWas is didnt seem to be needed but only a visual reference to the octal's permissions without doing the math. I will look deeper into where__ATTRis probally being called then. I'm not really worried with builds yet till I have a better understanging of the issue as I would expect all of them to fail at this point.
Never send out a patch that you have not at least test-built, unless you say you have not done so (and even then, it's a rare thing to do.)
thanks,
greg k-h
PATCH 1/1 staging: speakup: speakup_acntpc.c Consider octal permissions
Quoted 50 lines · Greg KH
Quoted 42 lines · Walt Feasel
Quoted 35 lines · Greg KH
Quoted 34 lines · Walt Feasel
Make suggested checkpatch modifications for
WARNING: Symbolic permissions 'S_IWUSR|S_IRUGO' are not preferred. Consider using octal permissions '0644'.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.Signed-off-by Walt Feasel <[email protected]>---
I am new to making trivial patches and do not make some for a few type of warnings.
This is one of them as I am not fully certain that it is as easy as this.
The replacing of 'S_IWUSR|S_IRUGO' with '0644' seems simple enough.
However the adding of '_RW' to '__ATTR' to make '__ATTR_RW' I saw in a reply to a patch and am not sure that it would be relevant in this case.
I also made a previous patch adding spaces around '|' and want to know if just replacing 'S_IWUSR|S_IRUGO' with '0644' in one shot would be acceptable since my being new and not fixing just one type of warning per patch. Seems straight forward but I have spammed other peoples email and the mailing list enough with improper patches.drivers/staging/speakup/speakup_acntpc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)drivers/staging/speakup/speakup_acntpc.c +0
-0@@ -56,28 +56,28 @@ static struct var_t vars[] = {/* These attributes will appear in /sys/accessibility/speakup/acntpc. */ Quoted 6 lines
drivers/staging/speakup/speakup_acntpc.c static struct var_t vars[] = { · line 59
Quoted 3 lines
drivers/staging/speakup/speakup_acntpc.c +1
-1static struct kobj_attribute caps_start_attribute = -__ATTR (caps_start,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_start,0644 , spk_var_show, spk_var_store);This breaks the build :(
Quoted 6 lines
I would assume from adding
_RWas is didnt seem to be needed but only a visual reference to the octal's permissions without doing the math. I will look deeper into where__ATTRis probally being called then. I'm not really worried with builds yet till I have a better understanging of the issue as I would expect all of them to fail at this point.
Quoted 6 lines
Never send out a patch that you have not at least test-built, unless you say you have not done so (and even then, it's a rare thing to do.)
thanks,
greg k-h
As I though was obvious in my note I was seeking clarification on a matter not to have a patch applied. If that were the case I would have sent it to the appropriate people and mailing list.
Quoted 1 line
From the Kernelnewbies main page, "Kernelnewbies is a community of aspiring
Linux kernel developers who work to improve their Kernels and more experienced developers willing to share their knowledge." So if this is not The proper channel what is?
Greg KH
maintainerPATCH 1/1 staging: speakup: speakup_acntpc.c Consider octal permissions
Quoted 57 lines · Walt Feasel
Quoted 50 lines · Greg KH
Quoted 42 lines · Walt Feasel
Quoted 35 lines · Greg KH
Quoted 34 lines · Walt Feasel
Make suggested checkpatch modifications for
WARNING: Symbolic permissions 'S_IWUSR|S_IRUGO' are not preferred. Consider using octal permissions '0644'.
WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider using octal permissions '0444'.Signed-off-by Walt Feasel <[email protected]>---
I am new to making trivial patches and do not make some for a few type of warnings.
This is one of them as I am not fully certain that it is as easy as this.
The replacing of 'S_IWUSR|S_IRUGO' with '0644' seems simple enough.
However the adding of '_RW' to '__ATTR' to make '__ATTR_RW' I saw in a reply to a patch and am not sure that it would be relevant in this case.
I also made a previous patch adding spaces around '|' and want to know if just replacing 'S_IWUSR|S_IRUGO' with '0644' in one shot would be acceptable since my being new and not fixing just one type of warning per patch. Seems straight forward but I have spammed other peoples email and the mailing list enough with improper patches.drivers/staging/speakup/speakup_acntpc.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)drivers/staging/speakup/speakup_acntpc.c +0
-0@@ -56,28 +56,28 @@ static struct var_t vars[] = {/* These attributes will appear in /sys/accessibility/speakup/acntpc. */ Quoted 6 lines
drivers/staging/speakup/speakup_acntpc.c static struct var_t vars[] = { · line 59
Quoted 3 lines
drivers/staging/speakup/speakup_acntpc.c +1
-1static struct kobj_attribute caps_start_attribute = -__ATTR (caps_start,S_IWUSR |S_IRUGO , spk_var_show, spk_var_store); +__ATTR_RW (caps_start,0644 , spk_var_show, spk_var_store);This breaks the build :(
Quoted 6 lines
I would assume from adding
_RWas is didnt seem to be needed but only a visual reference to the octal's permissions without doing the math. I will look deeper into where__ATTRis probally being called then. I'm not really worried with builds yet till I have a better understanging of the issue as I would expect all of them to fail at this point.Quoted 6 lines
Never send out a patch that you have not at least test-built, unless you say you have not done so (and even then, it's a rare thing to do.)
thanks,
greg k-h
Quoted 8 lines
As I though was obvious in my note I was seeking clarification on a matter not to have a patch applied. If that were the case I would have sent it to the appropriate people and mailing list.
From the Kernelnewbies main page, "Kernelnewbies is a community of aspiring Linux kernel developers who work to improve their Kernels and more experienced developers willing to share their knowledge." So if this is not The proper channel what is?
It is, but really, at least compile the code, the compiler will tell you that what you are suggesting is not correct much better than anyone else here can. Don't rely on others to do your basic work for you :)
greg k-h