Re: [PATCH] Staging: media: bcm2048: Use octal permissions '0444'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2016-10-18 at 14:36 +0200, Greg KH wrote:
> On Tue, Oct 18, 2016 at 05:52:21PM +0530, Shyam Saini wrote:
> > 
> > On Mon, 2016-10-17 at 21:37 +0200, Greg KH wrote:
> > > 
> > > On Mon, Oct 17, 2016 at 10:10:08PM +0530, Shyam Saini wrote:
> > > > 
> > > > 
> > > > On Mon, 2016-10-17 at 18:07 +0200, Greg KH wrote:
> > > > > 
> > > > > 
> > > > > On Mon, Oct 17, 2016 at 09:24:19PM +0530, Shyam Saini wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Fixed following checkpatch warning
> > > > > > Symbolic permissions 'S_IRUGO' are not preferred. Consider
> > > > > > using
> > > > > > octal
> > > > > > permissions '0444'
> > > > > > 
> > > > > > Signed-off-by: Shyam Saini <mayhs11saini@xxxxxxxxx>
> > > > > > ---
> > > > > >  drivers/staging/media/bcm2048/radio-bcm2048.c | 18
> > > > > > +++++++++
> > > > > > ----
> > > > > > -----
> > > > > >  1 file changed, 9 insertions(+), 9 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > b/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > index 5d2b702..9c286c3 100644
> > > > > > --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> > > > > > @@ -2103,21 +2103,21 @@ static struct device_attribute
> > > > > > attrs[]
> > > > > > = {
> > > > > >  	       bcm2048_rds_pi_match_write),
> > > > > >  	__ATTR(rds_wline, 0644, bcm2048_rds_wline_read,
> > > > > >  	       bcm2048_rds_wline_write),
> > > > > > -	__ATTR(rds_pi, S_IRUGO, bcm2048_rds_pi_read,
> > > > > > NULL),
> > > > > > -	__ATTR(rds_rt, S_IRUGO, bcm2048_rds_rt_read,
> > > > > > NULL),
> > > > > > -	__ATTR(rds_ps, S_IRUGO, bcm2048_rds_ps_read,
> > > > > > NULL),
> > > > > > -	__ATTR(fm_rds_flags, S_IRUGO,
> > > > > > bcm2048_fm_rds_flags_read,
> > > > > > NULL),
> > > > > > -	__ATTR(region_bottom_frequency, S_IRUGO,
> > > > > > +	__ATTR(rds_pi, 0444, bcm2048_rds_pi_read, NULL),
> > > > > > +	__ATTR(rds_rt, 0444, bcm2048_rds_rt_read, NULL),
> > > > > > +	__ATTR(rds_ps, 0444, bcm2048_rds_ps_read, NULL),
> > > > > > +	__ATTR(fm_rds_flags, 0444,
> > > > > > bcm2048_fm_rds_flags_read,
> > > > > > NULL),
> > > > > > +	__ATTR(region_bottom_frequency, 0444,
> > > > > >  	       bcm2048_region_bottom_frequency_read,
> > > > > > NULL),
> > > > > 
> > > > > 
> > > > > Why not use __ATTR_RO() for all of these instead?
> > > > I just used what checkpatch.pl suggested. Now I'll
> > > > use  __ATTR_RO().
> > > > 
> > > > Also shouldn't we change checkpatch.pl accordingly ?
> > > No, not all octal values should be converted that way.
> > > 
> > > checkpatch is a "hint", not always the exact thing that needs to
> > > be
> > > done, and sometimes it should be ignored.  Please always use your
> > > judgement.
> > I have used __ATTR_RO()  __ATTR_RW(). But, when I build it, i got
> > so
> > many error messages and build fails. I'm attaching a file
> > containing
> > errors.

> It's not always a drop-in replacement, as you have seen, you will
> need
> to rename some functions as well.
> 

I checked <linux/sysfs.h>.
What I can see is that __ATTR_RO append its argument with _show. I've
no clue how to fix it.
> The error messages should have been a big hint about this.  How good
> is
> your C knowledge?
I'm not an Expert in c.
I will scale my knowledge as intermediate level.

Thanks,
shyam


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux