[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