Re: [PATCH 15/18] ledtrig-blkdev: Add sysfs attributes to [un]link LEDs & devices

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

 



On Sat, Sep 04, 2021 at 05:35:29PM -0500, Ian Pilcher wrote:
> On 9/4/21 12:59 AM, Greg KH wrote:
> > DEVICE_ATTR_RO()?  Or something like that?  Do not use __ATTR() for
> > device attributes if at all possible, worst case, use DEVICE_ATTR()
> > here.
> 
> For some reason, it didn't click until now that these are device
> attributes (because I was focused on the fact that I was working on the
> LED trigger).
> 
> DEVICE_ATTR*() it is.
> 
> > And the mode settings are odd, are you sure you want that?
> 
> Yes.  These are write-only attributes.

DEVICE_ATTR_WO() then please.

> > > +	if (name_len == 0) {
> > > +		pr_info("blkdev LED: empty block device name\n");
> > 
> > Looks like debugging code, please remove.
> 
> It's really more of an error message for the system administrator.  So
> as with my earlier note, dev_info() would be my preference.

Nope, dev_err() for real errors please.

> > And how can this ever happen?
> 
> The blkdev_skip_space() and blkdev_find_space() calls effectively find
> the first non-whitespace token in the buffer (disk_name) and its length
> (name_len).  If the buffer only contains whitespace (e.g. echo > $ATTR),
> then name_len will be 0.

That's a crazy interface, as others pointed out, don't do that please.

thanks,

greg k-h



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux