[PATCH] Fix string to integer conversion
Jeremy Allison
jra at samba.org
Sat Feb 23 18:23:43 UTC 2019
On Sat, Feb 23, 2019 at 09:18:31AM +0100, Ralph Böhme wrote:
> > then you don't need to look inside, the
> > code is obvious.
>
> I'd still have to look to check why the initialisation is needed.
Well this kind of pattern (no-initialization
at declaration, followed by test after function
call) used to confuse Coverity into issuing false
positives, so I still think in this case initialization
at declaration is the right thing to do.
> > So I'm going to stick with my request, to make
> > future reviewers lives easier !
> >
> > You could help by reviewing the patch :-) :-).
>
> now you got my attention. :) I'd say the problem is, that the API of the new
> functions is still not ideal. I'd prefer
>
> bool tstrto[u]l[l](const char *nptr, char **endptr, int base, [unsigned][long]long int *result)
>
> Then the caller just does the usual
>
> ok = tstrtol(...);
> if (!ok) {
> ....
> }
>
> The function would return false for values that are out of range and
> probably also for the empty string and if there were no digits, I guess that
> is what all callers care about.
Oh god, what color is my bike shed today :-).
OK, that was my initial take on the API too, but
I was trying to use this as a teaching moment
and Swen was doing the work.
That makes it a different interface to the
existing POSIX strtoul()/strtoull() calls,
which I think Swen was trying to preserve
as a coding pattern.
I'm at Vault next week, so if you want it
changed I'll have to get to it later.
Jeremy.
More information about the samba-technical
mailing list