kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
From: waltfeasel@gmail.com (Walt Feasel)
To: kernelnewbies@lists.kernelnewbies.org
Subject: [TEST PATCH] staging: speakup: speakup_acntpc.c Consider octal permissions
Date: Sat, 26 Nov 2016 12:10:28 -0500	[thread overview]
Message-ID: <20161126171027.GC9754@localhost> (raw)
In-Reply-To: <20161126110513.GC8388@kroah.com>

On Sat, Nov 26, 2016 at 12:05:13PM +0100, Greg KH wrote:
> On Mon, Nov 21, 2016 at 10:03:00PM -0500, Walt Feasel wrote:
> > 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 <waltfeasel@gmail.com>
> > ---
> > 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(-)
> > 
> > diff --git a/drivers/staging/speakup/speakup_acntpc.c b/drivers/staging/speakup/speakup_acntpc.c
> > index 27f812e..51281be 100644
> > --- a/drivers/staging/speakup/speakup_acntpc.c
> > +++ b/drivers/staging/speakup/speakup_acntpc.c
> > @@ -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 :(
> 
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.

  reply	other threads:[~2016-11-26 17:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-22  3:03 [TEST PATCH] staging: speakup: speakup_acntpc.c Consider octal permissions Walt Feasel
2016-11-26 11:05 ` Greg KH
2016-11-26 17:10   ` Walt Feasel [this message]
2016-11-26 17:18     ` Greg KH
2016-11-26 18:11       ` Walt Feasel
2016-11-26 20:40         ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161126171027.GC9754@localhost \
    --to=waltfeasel@gmail.com \
    --cc=kernelnewbies@lists.kernelnewbies.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).