Messages in this thread | | | Date | Sun, 14 Oct 2018 20:53:46 +0200 | From | Christian Brauner <> | Subject | Re: [PATCH 1/2] sysctl: add overflow detection to proc_get_long() |
| |
On Sun, Oct 14, 2018 at 06:18:55PM +0100, Al Viro wrote: > On Sun, Oct 14, 2018 at 03:25:09PM +0200, Christian Brauner wrote: > > > +static unsigned long sysctl_strtoul_lenient(const char *cp, char **endp, > > + unsigned int base, bool *overflow) > > +{ > > + unsigned long long result; > > + unsigned int rv; > > + > > + cp = _parse_integer_fixup_radix(cp, &base); > > + rv = _parse_integer(cp, base, &result); > > + if ((rv & KSTRTOX_OVERFLOW) || > > + (result != (unsigned long long)(unsigned long)result)) > > + *overflow = true; > > + else > > + *overflow = false; > > Yecchh... First of all, the cast back to unsigned long long is completely > pointless. What's more,
Sorry, seriously asking: why? This was meant to handle the case where sizeof(unsigned long long) != sizeof(unsigned long) and I just looked at _kstrtoul() which does the same:
int _kstrtoul(const char *s, unsigned int base, unsigned long *res) { unsigned long long tmp; int rv;
rv = kstrtoull(s, base, &tmp); if (rv < 0) return rv; if (tmp != (unsigned long long)(unsigned long)tmp) return -ERANGE; *res = tmp; return 0; }
Sorry, if I'm being dense here.
> if (expr) > foo = true; > else > foo = flase; > is a fairly unidiomatic way to spell foo = expr; > > And... is there anything that would really care if this "overflow" thing had > been replaced by simply returning ~0UL on such? That would appear to be > a lot more natural API...
Yes, I thought about this but I really didn't want to risk breaking anything that relies on the weird old behavior. We can change it to that and assume that anything that doesn't explicitly set a maximum value wants to be capped at ULONG_MAX. Fine with me.
| |