diff mbox

[v2] Sunrpc: Supports hexadecimal number for sysctl files of sunrpc debug

Message ID 55F381CE.4070600@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kinglong Mee Sept. 12, 2015, 1:37 a.m. UTC
The sunrpc debug sysctl files only accept decimal number right now.
But all the XXXDBUG_XXX macros are defined as hexadecimal.
It is not easy to set or check an separate flag.

This patch let those files support accepting hexadecimal number,
(decimal number is also supported). Also, display it as hexadecimal.

v2,
Remove duplicate parsing of '0x...', just using simple_strtol(tmpbuf, &s, 0)
Fix a bug of isspace() checking after parsing

Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
 net/sunrpc/sysctl.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Comments

Trond Myklebust Nov. 3, 2015, 5:38 p.m. UTC | #1
On Fri, Sep 11, 2015 at 9:37 PM, Kinglong Mee <kinglongmee@gmail.com> wrote:
> The sunrpc debug sysctl files only accept decimal number right now.
> But all the XXXDBUG_XXX macros are defined as hexadecimal.
> It is not easy to set or check an separate flag.
>
> This patch let those files support accepting hexadecimal number,
> (decimal number is also supported). Also, display it as hexadecimal.
>
> v2,
> Remove duplicate parsing of '0x...', just using simple_strtol(tmpbuf, &s, 0)
> Fix a bug of isspace() checking after parsing
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
>  net/sunrpc/sysctl.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
> index 887f018..c88d9bc 100644
> --- a/net/sunrpc/sysctl.c
> +++ b/net/sunrpc/sysctl.c
> @@ -76,7 +76,7 @@ static int
>  proc_dodebug(struct ctl_table *table, int write,
>                                 void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> -       char            tmpbuf[20], c, *s;
> +       char            tmpbuf[20], c, *s = NULL;
>         char __user *p;
>         unsigned int    value;
>         size_t          left, len;
> @@ -103,23 +103,24 @@ proc_dodebug(struct ctl_table *table, int write,
>                         return -EFAULT;
>                 tmpbuf[left] = '\0';
>
> -               for (s = tmpbuf, value = 0; '0' <= *s && *s <= '9'; s++, left--)
> -                       value = 10 * value + (*s - '0');
> -               if (*s && !isspace(*s))
> -                       return -EINVAL;
> -               while (left && isspace(*s))
> -                       left--, s++;
> +               value = simple_strtol(tmpbuf, &s, 0);
> +               if (s) {
> +                       left -= (s - tmpbuf);
> +                       if (left && !isspace(*s))
> +                               return -EINVAL;
> +                       while (left && isspace(*s))
> +                               left--, s++;
> +               } else
> +                       left = 0;
>                 *(unsigned int *) table->data = value;
>                 /* Display the RPC tasks on writing to rpc_debug */
>                 if (strcmp(table->procname, "rpc_debug") == 0)
>                         rpc_show_tasks(&init_net);
>         } else {
> -               if (!access_ok(VERIFY_WRITE, buffer, left))
> -                       return -EFAULT;
> -               len = sprintf(tmpbuf, "%d", *(unsigned int *) table->data);
> +               len = sprintf(tmpbuf, "0x%04x", *(unsigned int *) table->data);
>                 if (len > left)
>                         len = left;
> -               if (__copy_to_user(buffer, tmpbuf, len))
> +               if (copy_to_user(buffer, tmpbuf, len))
>                         return -EFAULT;
>                 if ((left -= len) > 0) {
>                         if (put_user('\n', (char __user *)buffer + len))
> --
> 2.5.0
>

Bruce, did you take this patch, or is there an expectation that I should do so?

Cheers
  Trond
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
J. Bruce Fields Nov. 3, 2015, 7:32 p.m. UTC | #2
On Tue, Nov 03, 2015 at 12:38:19PM -0500, Trond Myklebust wrote:
> On Fri, Sep 11, 2015 at 9:37 PM, Kinglong Mee <kinglongmee@gmail.com> wrote:
> > The sunrpc debug sysctl files only accept decimal number right now.
> > But all the XXXDBUG_XXX macros are defined as hexadecimal.
> > It is not easy to set or check an separate flag.
> >
> > This patch let those files support accepting hexadecimal number,
> > (decimal number is also supported). Also, display it as hexadecimal.
> >
> > v2,
> > Remove duplicate parsing of '0x...', just using simple_strtol(tmpbuf, &s, 0)
> > Fix a bug of isspace() checking after parsing
> >
> > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> > ---
> >  net/sunrpc/sysctl.c | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
> > index 887f018..c88d9bc 100644
> > --- a/net/sunrpc/sysctl.c
> > +++ b/net/sunrpc/sysctl.c
> > @@ -76,7 +76,7 @@ static int
> >  proc_dodebug(struct ctl_table *table, int write,
> >                                 void __user *buffer, size_t *lenp, loff_t *ppos)
> >  {
> > -       char            tmpbuf[20], c, *s;
> > +       char            tmpbuf[20], c, *s = NULL;
> >         char __user *p;
> >         unsigned int    value;
> >         size_t          left, len;
> > @@ -103,23 +103,24 @@ proc_dodebug(struct ctl_table *table, int write,
> >                         return -EFAULT;
> >                 tmpbuf[left] = '\0';
> >
> > -               for (s = tmpbuf, value = 0; '0' <= *s && *s <= '9'; s++, left--)
> > -                       value = 10 * value + (*s - '0');
> > -               if (*s && !isspace(*s))
> > -                       return -EINVAL;
> > -               while (left && isspace(*s))
> > -                       left--, s++;
> > +               value = simple_strtol(tmpbuf, &s, 0);
> > +               if (s) {
> > +                       left -= (s - tmpbuf);
> > +                       if (left && !isspace(*s))
> > +                               return -EINVAL;
> > +                       while (left && isspace(*s))
> > +                               left--, s++;
> > +               } else
> > +                       left = 0;
> >                 *(unsigned int *) table->data = value;
> >                 /* Display the RPC tasks on writing to rpc_debug */
> >                 if (strcmp(table->procname, "rpc_debug") == 0)
> >                         rpc_show_tasks(&init_net);
> >         } else {
> > -               if (!access_ok(VERIFY_WRITE, buffer, left))
> > -                       return -EFAULT;
> > -               len = sprintf(tmpbuf, "%d", *(unsigned int *) table->data);
> > +               len = sprintf(tmpbuf, "0x%04x", *(unsigned int *) table->data);
> >                 if (len > left)
> >                         len = left;
> > -               if (__copy_to_user(buffer, tmpbuf, len))
> > +               if (copy_to_user(buffer, tmpbuf, len))
> >                         return -EFAULT;
> >                 if ((left -= len) > 0) {
> >                         if (put_user('\n', (char __user *)buffer + len))
> > --
> > 2.5.0
> >
> 
> Bruce, did you take this patch, or is there an expectation that I should do so?

Patch looked OK to me, but I didn't take it, could you?

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Nov. 3, 2015, 7:49 p.m. UTC | #3
On Tue, Nov 3, 2015 at 2:32 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Tue, Nov 03, 2015 at 12:38:19PM -0500, Trond Myklebust wrote:
>> On Fri, Sep 11, 2015 at 9:37 PM, Kinglong Mee <kinglongmee@gmail.com> wrote:
>> > The sunrpc debug sysctl files only accept decimal number right now.
>> > But all the XXXDBUG_XXX macros are defined as hexadecimal.
>> > It is not easy to set or check an separate flag.
>> >
>> > This patch let those files support accepting hexadecimal number,
>> > (decimal number is also supported). Also, display it as hexadecimal.
>> >
>> > v2,
>> > Remove duplicate parsing of '0x...', just using simple_strtol(tmpbuf, &s, 0)
>> > Fix a bug of isspace() checking after parsing
>> >
>> > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> > ---
>> >  net/sunrpc/sysctl.c | 23 ++++++++++++-----------
>> >  1 file changed, 12 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
>> > index 887f018..c88d9bc 100644
>> > --- a/net/sunrpc/sysctl.c
>> > +++ b/net/sunrpc/sysctl.c
>> > @@ -76,7 +76,7 @@ static int
>> >  proc_dodebug(struct ctl_table *table, int write,
>> >                                 void __user *buffer, size_t *lenp, loff_t *ppos)
>> >  {
>> > -       char            tmpbuf[20], c, *s;
>> > +       char            tmpbuf[20], c, *s = NULL;
>> >         char __user *p;
>> >         unsigned int    value;
>> >         size_t          left, len;
>> > @@ -103,23 +103,24 @@ proc_dodebug(struct ctl_table *table, int write,
>> >                         return -EFAULT;
>> >                 tmpbuf[left] = '\0';
>> >
>> > -               for (s = tmpbuf, value = 0; '0' <= *s && *s <= '9'; s++, left--)
>> > -                       value = 10 * value + (*s - '0');
>> > -               if (*s && !isspace(*s))
>> > -                       return -EINVAL;
>> > -               while (left && isspace(*s))
>> > -                       left--, s++;
>> > +               value = simple_strtol(tmpbuf, &s, 0);
>> > +               if (s) {
>> > +                       left -= (s - tmpbuf);
>> > +                       if (left && !isspace(*s))
>> > +                               return -EINVAL;
>> > +                       while (left && isspace(*s))
>> > +                               left--, s++;
>> > +               } else
>> > +                       left = 0;
>> >                 *(unsigned int *) table->data = value;
>> >                 /* Display the RPC tasks on writing to rpc_debug */
>> >                 if (strcmp(table->procname, "rpc_debug") == 0)
>> >                         rpc_show_tasks(&init_net);
>> >         } else {
>> > -               if (!access_ok(VERIFY_WRITE, buffer, left))
>> > -                       return -EFAULT;
>> > -               len = sprintf(tmpbuf, "%d", *(unsigned int *) table->data);
>> > +               len = sprintf(tmpbuf, "0x%04x", *(unsigned int *) table->data);
>> >                 if (len > left)
>> >                         len = left;
>> > -               if (__copy_to_user(buffer, tmpbuf, len))
>> > +               if (copy_to_user(buffer, tmpbuf, len))
>> >                         return -EFAULT;
>> >                 if ((left -= len) > 0) {
>> >                         if (put_user('\n', (char __user *)buffer + len))
>> > --
>> > 2.5.0
>> >
>>
>> Bruce, did you take this patch, or is there an expectation that I should do so?
>
> Patch looked OK to me, but I didn't take it, could you?
>

Sure. I'll pick it up.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c
index 887f018..c88d9bc 100644
--- a/net/sunrpc/sysctl.c
+++ b/net/sunrpc/sysctl.c
@@ -76,7 +76,7 @@  static int
 proc_dodebug(struct ctl_table *table, int write,
 				void __user *buffer, size_t *lenp, loff_t *ppos)
 {
-	char		tmpbuf[20], c, *s;
+	char		tmpbuf[20], c, *s = NULL;
 	char __user *p;
 	unsigned int	value;
 	size_t		left, len;
@@ -103,23 +103,24 @@  proc_dodebug(struct ctl_table *table, int write,
 			return -EFAULT;
 		tmpbuf[left] = '\0';
 
-		for (s = tmpbuf, value = 0; '0' <= *s && *s <= '9'; s++, left--)
-			value = 10 * value + (*s - '0');
-		if (*s && !isspace(*s))
-			return -EINVAL;
-		while (left && isspace(*s))
-			left--, s++;
+		value = simple_strtol(tmpbuf, &s, 0);
+		if (s) {
+			left -= (s - tmpbuf);
+			if (left && !isspace(*s))
+				return -EINVAL;
+			while (left && isspace(*s))
+				left--, s++;
+		} else
+			left = 0;
 		*(unsigned int *) table->data = value;
 		/* Display the RPC tasks on writing to rpc_debug */
 		if (strcmp(table->procname, "rpc_debug") == 0)
 			rpc_show_tasks(&init_net);
 	} else {
-		if (!access_ok(VERIFY_WRITE, buffer, left))
-			return -EFAULT;
-		len = sprintf(tmpbuf, "%d", *(unsigned int *) table->data);
+		len = sprintf(tmpbuf, "0x%04x", *(unsigned int *) table->data);
 		if (len > left)
 			len = left;
-		if (__copy_to_user(buffer, tmpbuf, len))
+		if (copy_to_user(buffer, tmpbuf, len))
 			return -EFAULT;
 		if ((left -= len) > 0) {
 			if (put_user('\n', (char __user *)buffer + len))