On Thu 2021-02-11 13:55:26, Petr Mladek wrote:
On Mon 2021-02-08 17:38:29, Richard Fitzgerald wrote:
On 08/02/2021 15:18, Andy Shevchenko wrote:
On Mon, Feb 08, 2021 at 02:01:52PM +0000, Richard Fitzgerald wrote: A nit-pick: What if we rewrite above as
static unsigned long long simple_strntoull(const char *cp, size_t max_chars, char **endp, unsigned int base) { unsigned long long result = 0ULL; const char *startp = cp; unsigned int rv; size_t chars;
cp = _parse_integer_fixup_radix(cp, &base); chars = cp - startp; if (chars >= max_chars) { /* We hit the limit */ cp = startp + max_chars; } else { rv = _parse_integer_limit(cp, base, &result, max_chars - chars); /* FIXME */ cp += (rv & ~KSTRTOX_OVERFLOW); }
if (endp) *endp = (char *)cp;
return result; }
...
I don't mind rewriting that code if you prefer that way. I am used to working on other kernel subsytems where the preference is to bail out on the error case so that the "normal" case flows without nesting.
Yeah. But in this case Andy's variant looks slightly better redable to me.
...
val.s = simple_strntoll(str,field_width > 0 ? field_width : SIZE_MAX,&next, base);is? Also, is field_width == 0 should be treated as "parse to the MAX"?
Earlier code terminates scanning if the width parsed from the format string is <= 0.
So field_width can only be -1 or > 0 here. But now you point it out, that test would be better as field_width >= 0 ... so it deals with 0 if it ever happened to sneak through to here somehow.
It might make sense to be proactive and change it to >= 0. But I would do it in a separate patch. The "< 0" condition matches the original code.
Ah, I have missed that you have already sent v6 where you did this change in the same patch. There is no need to resend it just because of this. I am going to look at v6.
Best Regards, Petr