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
