| Message ID | 55F17A7E.50406@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 10, 2015 at 08:41:34PM +0800, Kinglong Mee 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. That potentially breaks backwards-compatibility if an program that reads this file isn't prepared to accept a hexadecimal value. That said, rpcdebug seems OK (it uses strtoul(.,.,0), which I think should parse that fine?), and it may well be the only program that actually parses this, so.... I suppose it's OK with me if it's OK with Trond. --b. > > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> > --- > net/sunrpc/sysctl.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c > index 887f018..8e85e6f 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,27 @@ 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++; > + if (tmpbuf[0] == '0' && tmpbuf[1] == 'x') > + value = simple_strtol(tmpbuf, &s, 16); > + else > + value = simple_strtol(tmpbuf, &s, 0); > + if (s) { > + if (!isspace(*s)) > + return -EINVAL; > + left -= (s - tmpbuf); > + 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 -- 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
On 9/12/2015 05:23, J. Bruce Fields wrote: > On Thu, Sep 10, 2015 at 08:41:34PM +0800, Kinglong Mee 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. > > That potentially breaks backwards-compatibility if an program that reads > this file isn't prepared to accept a hexadecimal value. > > That said, rpcdebug seems OK (it uses strtoul(.,.,0), which I think > should parse that fine?), and it may well be the only program that > actually parses this, so.... I suppose it's OK with me if it's OK with > Trond. Thanks for your comments. I have test of rpcdebug, it's OK when showing the debug flags. > > --b. > >> >> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> >> --- >> net/sunrpc/sysctl.c | 26 +++++++++++++++----------- >> 1 file changed, 15 insertions(+), 11 deletions(-) >> >> diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c >> index 887f018..8e85e6f 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,27 @@ 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++; >> + if (tmpbuf[0] == '0' && tmpbuf[1] == 'x') >> + value = simple_strtol(tmpbuf, &s, 16); It's a duplicate of this. >> + else >> + value = simple_strtol(tmpbuf, &s, 0); >> + if (s) { >> + if (!isspace(*s)) It's a bug of checking '\0' here. I will fix those problems in new version. thanks, Kinglong Mee >> + return -EINVAL; >> + left -= (s - tmpbuf); >> + 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 > -- 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 --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c index 887f018..8e85e6f 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,27 @@ 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++; + if (tmpbuf[0] == '0' && tmpbuf[1] == 'x') + value = simple_strtol(tmpbuf, &s, 16); + else + value = simple_strtol(tmpbuf, &s, 0); + if (s) { + if (!isspace(*s)) + return -EINVAL; + left -= (s - tmpbuf); + 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))
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. Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> --- net/sunrpc/sysctl.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)