Re: [PATCH v2] staging: wilc1000: Process WARN, INFO options of debug levels from user

Greg KH
2015-08-15T02:06:15Z

On Fri, Aug 14, 2015 at 10:50:07PM +0530, Chandra S Gorentla wrote:
> This patch enables setting the module's debug options WARN and INFO in the
> debugfs file 'wilc_debug_level'. This enables the user to enable logging
> of warning and other information. Before this change writes to this
> debugfs file sets only one option DGB. This is additional to the default
> option ERR.
>
> As a side effect, this patch removes the 'sparse' warning -
> 'warning: incorrect type in argument 2 (different address spaces)'.
>
> Signed-off-by: Chandra S Gorentla <csgorentla@gmail.com>
> ---
> drivers/staging/wilc1000/wilc_debugfs.c | 28 +++++++++-------------------
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_debugfs.c b/drivers/staging/wilc1000/wilc_debugfs.c
> index be2e901..23419d9 100644
> --- a/drivers/staging/wilc1000/wilc_debugfs.c
> +++ b/drivers/staging/wilc1000/wilc_debugfs.c
> @@ -48,29 +48,18 @@ static ssize_t wilc_debug_level_read(struct file *file, char __user *userbuf, si
> return simple_read_from_buffer(userbuf, count, ppos, buf, res);
> }
>
> -static ssize_t wilc_debug_level_write(struct file *filp, const char *buf, size_t count, loff_t *ppos)
> +static ssize_t wilc_debug_level_write(struct file *filp, const char __user *buf,
> + size_t count, loff_t *ppos)
> {
> - char buffer[128] = {};
> int flag = 0;
> + int ret;
>
> - if (count > sizeof(buffer))
> - return -EINVAL;
> -
> - if (copy_from_user(buffer, buf, count)) {
> - return -EFAULT;
> - }
> -
> - flag = buffer[0] - '0';
> -
> - if (flag > 0)
> - flag = DEBUG | ERR;
> - else if (flag < 0)
> - flag = 100;
> + ret = kstrtouint_from_user(buf, count, 16, &flag);
> + if (ret)
> + return ret;
>
> - if (flag > DBG_LEVEL_ALL) {
> - printk("%s, value (0x%08x) is out of range, stay previous flag (0x%08x)\n", __func__, flag, atomic_read(&DEBUG_LEVEL));

Why did you remove this warning value?

> - return -EFAULT;

I agree, this should be changed to -EINVAL like you did, but please put
back the printk for now.

thanks,

greg k-h