Re: [PATCH 17/18] ledtrig-blkdev: Add mode (read/write/rw) sysfs attributue

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

 



On Fri, Sep 03, 2021 at 03:45:47PM -0500, Ian Pilcher wrote:
> +static ssize_t blkdev_mode_store(struct device *const dev,
> +				 struct device_attribute *const attr,
> +				 const char *const buf, const size_t count)
> +{
> +	struct ledtrig_blkdev_led *const led = led_trigger_get_drvdata(dev);
> +	const char *const mode_name = blkdev_skip_space(buf);
> +	const char *const endp = blkdev_find_space(mode_name);
> +	const ptrdiff_t name_len = endp - mode_name;	/* always >= 0 */
> +	enum ledtrig_blkdev_mode mode;
> +
> +	if (name_len == 0) {
> +		pr_info("blkdev LED: empty mode\n");
> +		return -EINVAL;
> +	}
> +
> +	for (mode = LEDTRIG_BLKDEV_MODE_RO;
> +				mode <= LEDTRIG_BLKDEV_MODE_RW; ++mode) {
> +
> +		if (ledtrig_blkdev_streq(blkdev_modes[mode].name,
> +						mode_name, name_len)) {
> +			WRITE_ONCE(led->mode, mode);
> +			return count;
> +		}
> +	}
> +
> +	pr_info("blkdev LED: invalid mode (%.*s)\n", (int)name_len, mode_name);

Please do not use pr_* calls in a driver when you have a struct device.

Also, you are now allowing any user to spam the kernel log, this shold
be a dev_dbg() call at the most, if it is even needed at all.  Same for
the other pr_info() call in this function, please remove them all.

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