| Message ID | 1341442827-21339-2-git-send-email-eldad@fogrefinery.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2012-07-05 at 01:00 +0200, Eldad Zack wrote: > This patch replaces the usage of simple_strtoul with kstrtoint in > get_int(). Just some trivia: > diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h [] > @@ -217,7 +217,7 @@ extern int qword_get(char **bpp, char *dest, int bufsize); > static inline int get_int(char **bpp, int *anint) > { > char buf[50]; > - char *ep; > + ssize_t ret; > int rv; > int len = qword_get(bpp, buf, 50); This would be nicer as int len = qword_get(bpp, buf, sizeof(buf)); > @@ -226,8 +226,8 @@ static inline int get_int(char **bpp, int *anint) > if (len == 0) > return -ENOENT; > > - rv = simple_strtol(buf, &ep, 0); > - if (*ep) > + ret = kstrtoint(buf, 0, &rv); > + if (ret) > return -EINVAL; > > *anint = rv; ret and rv aren't useful anymore. Perhaps: if (kstrtoint(buf, 0, anint)) return -EINVAL; return 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 Thu, Jul 05, 2012 at 01:00:26AM +0200, Eldad Zack wrote: > This patch replaces the usage of simple_strtoul with kstrtoint in > get_int(). Why? --b. > > Signed-off-by: Eldad Zack <eldad@fogrefinery.com> > --- > include/linux/sunrpc/cache.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h > index 0757887..9a60bcc 100644 > --- a/include/linux/sunrpc/cache.h > +++ b/include/linux/sunrpc/cache.h > @@ -217,7 +217,7 @@ extern int qword_get(char **bpp, char *dest, int bufsize); > static inline int get_int(char **bpp, int *anint) > { > char buf[50]; > - char *ep; > + ssize_t ret; > int rv; > int len = qword_get(bpp, buf, 50); > > @@ -226,8 +226,8 @@ static inline int get_int(char **bpp, int *anint) > if (len == 0) > return -ENOENT; > > - rv = simple_strtol(buf, &ep, 0); > - if (*ep) > + ret = kstrtoint(buf, 0, &rv); > + if (ret) > return -EINVAL; > > *anint = rv; > -- > 1.7.10 > -- 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 Wed, 4 Jul 2012, Joe Perches wrote: > On Thu, 2012-07-05 at 01:00 +0200, Eldad Zack wrote: > > This patch replaces the usage of simple_strtoul with kstrtoint in > > get_int(). > > > int len = qword_get(bpp, buf, 50); > This would be nicer as > int len = qword_get(bpp, buf, sizeof(buf)); I agree, thanks! > > @@ -226,8 +226,8 @@ static inline int get_int(char **bpp, int *anint) > > + ret = kstrtoint(buf, 0, &rv); > > + if (ret) > > return -EINVAL; > > > > *anint = rv; > ret and rv aren't useful anymore. > > Perhaps: > > if (kstrtoint(buf, 0, anint)) > return -EINVAL; > > return 0; Yes, that works better, since kstrtoint() will not write to anint unless it's successful, so the whole business with rv is indeed redundant. Looks much neater now thanks to your comments - I will resend it a bit later. Should I add you as Signed-off-by? Eldad -- 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 Wed, 4 Jul 2012, J. Bruce Fields wrote: > On Thu, Jul 05, 2012 at 01:00:26AM +0200, Eldad Zack wrote: > > This patch replaces the usage of simple_strtoul with kstrtoint in > > get_int(). > > Why? Since the simple_str* family don't account for overflow and AFAIK deprecated. Also, in this specific case, the long from strtol is silently converted to an int by the caller. Eldad -- 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 Thu, Jul 05, 2012 at 09:04:03PM +0200, Eldad Zack wrote: > > On Wed, 4 Jul 2012, J. Bruce Fields wrote: > > On Thu, Jul 05, 2012 at 01:00:26AM +0200, Eldad Zack wrote: > > > This patch replaces the usage of simple_strtoul with kstrtoint in > > > get_int(). > > > > Why? > > Since the simple_str* family don't account for overflow and AFAIK > deprecated. Also, in this specific case, the long from strtol is > silently converted to an int by the caller. OK, thanks. Please use that as the changelog when you resend. ("This patch replaces..." tells me nothing I couldn't figure out from looking at the patch.) --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
On Thu, 5 Jul 2012, J. Bruce Fields wrote: > On Thu, Jul 05, 2012 at 09:04:03PM +0200, Eldad Zack wrote: > > > > On Wed, 4 Jul 2012, J. Bruce Fields wrote: > > > On Thu, Jul 05, 2012 at 01:00:26AM +0200, Eldad Zack wrote: > > > > This patch replaces the usage of simple_strtoul with kstrtoint in > > > > get_int(). > > > > > > Why? > > > > Since the simple_str* family don't account for overflow and AFAIK > > deprecated. Also, in this specific case, the long from strtol is > > silently converted to an int by the caller. > > OK, thanks. Please use that as the changelog when you resend. ("This > patch replaces..." tells me nothing I couldn't figure out from looking > at the patch.) Will do, sorry about that. Eldad -- 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/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h index 0757887..9a60bcc 100644 --- a/include/linux/sunrpc/cache.h +++ b/include/linux/sunrpc/cache.h @@ -217,7 +217,7 @@ extern int qword_get(char **bpp, char *dest, int bufsize); static inline int get_int(char **bpp, int *anint) { char buf[50]; - char *ep; + ssize_t ret; int rv; int len = qword_get(bpp, buf, 50); @@ -226,8 +226,8 @@ static inline int get_int(char **bpp, int *anint) if (len == 0) return -ENOENT; - rv = simple_strtol(buf, &ep, 0); - if (*ep) + ret = kstrtoint(buf, 0, &rv); + if (ret) return -EINVAL; *anint = rv;
This patch replaces the usage of simple_strtoul with kstrtoint in get_int(). Signed-off-by: Eldad Zack <eldad@fogrefinery.com> --- include/linux/sunrpc/cache.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)