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
