* [PATCH 2/5] net/sunrpc: use kstrtoul, etc [not found] <1320586010-21931-1-git-send-email-julia@diku.dk> @ 2011-11-06 13:26 ` Julia Lawall 2011-11-08 19:38 ` Alexey Dobriyan 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2011-11-06 13:26 UTC (permalink / raw) To: J. Bruce Fields Cc: kernel-janitors, Neil Brown, Trond Myklebust, David S. Miller, linux-nfs, netdev, linux-kernel From: Julia Lawall <julia@diku.dk> Use kstrtoul, etc instead of the now deprecated strict_strtoul, etc. A semantic patch rule for the kstrtoul case is as follows: (http://coccinelle.lip6.fr/) // <smpl> @@ expression a,b; {int,long} *c; @@ -strict_strtoul +kstrtoul (a,b,c) // </smpl> Signed-off-by: Julia Lawall <julia@diku.dk> --- net/sunrpc/addr.c | 6 +++--- net/sunrpc/auth.c | 2 +- net/sunrpc/xprtsock.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff -u -p a/net/sunrpc/addr.c b/net/sunrpc/addr.c --- a/net/sunrpc/addr.c +++ b/net/sunrpc/addr.c @@ -182,7 +182,7 @@ static int rpc_parse_scope_id(const char scope_id = dev->ifindex; dev_put(dev); } else { - if (strict_strtoul(p, 10, &scope_id) == 0) { + if (kstrtoul(p, 10, &scope_id) == 0) { kfree(p); return 0; } @@ -322,7 +322,7 @@ size_t rpc_uaddr2sockaddr(const char *ua c = strrchr(buf, '.'); if (unlikely(c == NULL)) return 0; - if (unlikely(strict_strtoul(c + 1, 10, &portlo) != 0)) + if (unlikely(kstrtoul(c + 1, 10, &portlo) != 0)) return 0; if (unlikely(portlo > 255)) return 0; @@ -331,7 +331,7 @@ size_t rpc_uaddr2sockaddr(const char *ua c = strrchr(buf, '.'); if (unlikely(c == NULL)) return 0; - if (unlikely(strict_strtoul(c + 1, 10, &porthi) != 0)) + if (unlikely(kstrtoul(c + 1, 10, &porthi) != 0)) return 0; if (unlikely(porthi > 255)) return 0; diff -u -p a/net/sunrpc/auth.c b/net/sunrpc/auth.c --- a/net/sunrpc/auth.c +++ b/net/sunrpc/auth.c @@ -47,7 +47,7 @@ static int param_set_hashtbl_sz(const ch if (!val) goto out_inval; - ret = strict_strtoul(val, 0, &num); + ret = kstrtoul(val, 0, &num); if (ret == -EINVAL) goto out_inval; nbits = fls(num); diff -u -p a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -2925,7 +2925,7 @@ static int param_set_uint_minmax(const c if (!val) return -EINVAL; - ret = strict_strtoul(val, 0, &num); + ret = kstrtoul(val, 0, &num); if (ret == -EINVAL || num < min || num > max) return -EINVAL; *((unsigned int *)kp->arg) = num; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] net/sunrpc: use kstrtoul, etc 2011-11-06 13:26 ` [PATCH 2/5] net/sunrpc: use kstrtoul, etc Julia Lawall @ 2011-11-08 19:38 ` Alexey Dobriyan 2011-11-08 20:19 ` Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Alexey Dobriyan @ 2011-11-08 19:38 UTC (permalink / raw) To: Julia Lawall Cc: J. Bruce Fields, kernel-janitors, Neil Brown, Trond Myklebust, David S. Miller, linux-nfs, netdev, linux-kernel On Sun, Nov 06, 2011 at 02:26:47PM +0100, Julia Lawall wrote: > @@ > expression a,b; > {int,long} *c; > @@ > > -strict_strtoul > +kstrtoul No, no, no! In every case see the type or real data and use appropriate function. kstrtou8() for ports. This program creates lots of bogus patches in this case. > --- a/net/sunrpc/addr.c > +++ b/net/sunrpc/addr.c > @@ -322,7 +322,7 @@ size_t rpc_uaddr2sockaddr(const char *ua > c = strrchr(buf, '.'); > if (unlikely(c == NULL)) > return 0; > - if (unlikely(strict_strtoul(c + 1, 10, &portlo) != 0)) > + if (unlikely(kstrtoul(c + 1, 10, &portlo) != 0)) > return 0; > if (unlikely(portlo > 255)) > return 0; > @@ -331,7 +331,7 @@ size_t rpc_uaddr2sockaddr(const char *ua > c = strrchr(buf, '.'); > if (unlikely(c == NULL)) > return 0; > - if (unlikely(strict_strtoul(c + 1, 10, &porthi) != 0)) > + if (unlikely(kstrtoul(c + 1, 10, &porthi) != 0)) > return 0; > if (unlikely(porthi > 255)) > return 0; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] net/sunrpc: use kstrtoul, etc 2011-11-08 19:38 ` Alexey Dobriyan @ 2011-11-08 20:19 ` Julia Lawall 2011-11-08 20:45 ` Alexey Dobriyan 2011-11-09 6:15 ` Julia Lawall 0 siblings, 2 replies; 5+ messages in thread From: Julia Lawall @ 2011-11-08 20:19 UTC (permalink / raw) To: Alexey Dobriyan Cc: Julia Lawall, J. Bruce Fields, kernel-janitors, Neil Brown, Trond Myklebust, David S. Miller, linux-nfs, netdev, linux-kernel On Tue, 8 Nov 2011, Alexey Dobriyan wrote: > On Sun, Nov 06, 2011 at 02:26:47PM +0100, Julia Lawall wrote: >> @@ >> expression a,b; >> {int,long} *c; >> @@ >> >> -strict_strtoul >> +kstrtoul > > No, no, no! Sorry, this was not the real rule I used for the strtoul case. Instead I used the following: @@ typedef ulong; expression a,b; {ulong,unsigned long,unsigned int,size_t} *c; @@ -strict_strtoul +kstrtoul (a,b,c) But now I have seen that there is a separate function for integers, so I have made a rule to use that function when the type is unsigned int. > In every case see the type or real data and use appropriate function. > kstrtou8() for ports. The type of the destination variable in all of these cases is unsigned long. But maybe that is not enough information to make the transformation in the right way. julia > This program creates lots of bogus patches in this case. > >> --- a/net/sunrpc/addr.c >> +++ b/net/sunrpc/addr.c >> @@ -322,7 +322,7 @@ size_t rpc_uaddr2sockaddr(const char *ua >> c = strrchr(buf, '.'); >> if (unlikely(c == NULL)) >> return 0; >> - if (unlikely(strict_strtoul(c + 1, 10, &portlo) != 0)) >> + if (unlikely(kstrtoul(c + 1, 10, &portlo) != 0)) >> return 0; >> if (unlikely(portlo > 255)) >> return 0; >> @@ -331,7 +331,7 @@ size_t rpc_uaddr2sockaddr(const char *ua >> c = strrchr(buf, '.'); >> if (unlikely(c == NULL)) >> return 0; >> - if (unlikely(strict_strtoul(c + 1, 10, &porthi) != 0)) >> + if (unlikely(kstrtoul(c + 1, 10, &porthi) != 0)) >> return 0; >> if (unlikely(porthi > 255)) >> return 0; > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] net/sunrpc: use kstrtoul, etc 2011-11-08 20:19 ` Julia Lawall @ 2011-11-08 20:45 ` Alexey Dobriyan 2011-11-09 6:15 ` Julia Lawall 1 sibling, 0 replies; 5+ messages in thread From: Alexey Dobriyan @ 2011-11-08 20:45 UTC (permalink / raw) To: Julia Lawall Cc: Julia Lawall, J. Bruce Fields, kernel-janitors, Neil Brown, Trond Myklebust, David S. Miller, linux-nfs, netdev, linux-kernel On Tue, Nov 08, 2011 at 09:19:30PM +0100, Julia Lawall wrote: > > > On Tue, 8 Nov 2011, Alexey Dobriyan wrote: > > > On Sun, Nov 06, 2011 at 02:26:47PM +0100, Julia Lawall wrote: > >> @@ > >> expression a,b; > >> {int,long} *c; > >> @@ > >> > >> -strict_strtoul > >> +kstrtoul > > > > No, no, no! > > Sorry, this was not the real rule I used for the strtoul case. Instead I > used the following: > > @@ > typedef ulong; > expression a,b; > {ulong,unsigned long,unsigned int,size_t} *c; > @@ > > -strict_strtoul > +kstrtoul > (a,b,c) > > But now I have seen that there is a separate function for integers, so I > have made a rule to use that function when the type is unsigned int. > > > In every case see the type or real data and use appropriate function. > > kstrtou8() for ports. > > The type of the destination variable in all of these cases is unsigned > long. But maybe that is not enough information to make the > transformation in the right way. That's because previous functions following libc didn't accept anything less than unsigned long. For these conversion, one should literally look at every usecase and see what types data have for real (not unsigned long) and make conversion and remove explicit EINVAL checks if necesasry. In sunrpc case: switch to kstrtou8 + remove "> 255" check. This program doesn't and won't do that. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/5] net/sunrpc: use kstrtoul, etc 2011-11-08 20:19 ` Julia Lawall 2011-11-08 20:45 ` Alexey Dobriyan @ 2011-11-09 6:15 ` Julia Lawall 1 sibling, 0 replies; 5+ messages in thread From: Julia Lawall @ 2011-11-09 6:15 UTC (permalink / raw) To: Alexey Dobriyan Cc: J. Bruce Fields, kernel-janitors, Neil Brown, Trond Myklebust, David S. Miller, linux-nfs, netdev, linux-kernel In looking through some examples, I see, e.g.: if (strict_strtoul(buf, 10, &val) < 0) return -EINVAL; if (val < 1 || val > 2) return -EINVAL; In this case the only valid values are 1 and 2, which are much smaller than the u8 range. Is it useful to use kstrtou8 anyway? I see that kstrtou8 returns -ERANGE not -EINVAL when the value is out of bounds. If kstrtou8 is to be used, should the subsequent if (val < 1 || val > 2) now return -ERANGE to be consistent? julia ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-09 6:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1320586010-21931-1-git-send-email-julia@diku.dk>
2011-11-06 13:26 ` [PATCH 2/5] net/sunrpc: use kstrtoul, etc Julia Lawall
2011-11-08 19:38 ` Alexey Dobriyan
2011-11-08 20:19 ` Julia Lawall
2011-11-08 20:45 ` Alexey Dobriyan
2011-11-09 6:15 ` Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox