Neaten code style in get_int()
Signed-off-by: Eldad Zack <[email protected]>
---
include/linux/sunrpc/cache.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index f5fd616..0757887 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -220,10 +220,16 @@ static inline int get_int(char **bpp, int *anint)
char *ep;
int rv;
int len = qword_get(bpp, buf, 50);
- if (len < 0) return -EINVAL;
- if (len ==0) return -ENOENT;
+
+ if (len < 0)
+ return -EINVAL;
+ if (len == 0)
+ return -ENOENT;
+
rv = simple_strtol(buf, &ep, 0);
- if (*ep) return -EINVAL;
+ if (*ep)
+ return -EINVAL;
+
*anint = rv;
return 0;
}
--
1.7.10
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
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 <[email protected]>
> ---
> 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
>
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;
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.
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
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
This patch replaces the usage of simple_strtoul with kstrtoint in
get_int().
Signed-off-by: Eldad Zack <[email protected]>
---
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