From mboxrd@z Thu Jan  1 00:00:00 1970
From: waltfeasel@gmail.com (Walt Feasel)
Date: Sat, 26 Nov 2016 12:10:28 -0500
Subject: [TEST PATCH] staging: speakup: speakup_acntpc.c Consider octal
	permissions
In-Reply-To: <20161126110513.GC8388@kroah.com>
References: <1479783780-24229-1-git-send-email-waltfeasel@gmail.com>
	<20161126110513.GC8388@kroah.com>
Message-ID: <20161126171027.GC9754@localhost>
To: kernelnewbies@lists.kernelnewbies.org
List-Id: kernelnewbies.lists.kernelnewbies.org

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.