[PATCH] Fix string to integer conversion

Ralph Böhme slow at samba.org
Sat Feb 23 08:18:31 UTC 2019


On Fri, Feb 22, 2019 at 03:48:40PM -0800, Jeremy Allison wrote:
>On Fri, Feb 22, 2019 at 10:51:43PM +0100, Ralph Böhme via samba-technical wrote:
>> this goes for pointers, not for non-pointers. Imho the latter should only be
>> initialized when needed.
>
>Yeah, but the code does:
>
>int error;
>
>stuff_call_here(var1, var2, &error);
>
>if (error != 0) {
>	..
>}
>
>So just reading the code, you
>would then have to go look inside
>
>stuff_call_here()
>
>to see exactly what gets done
>on success.
>
>If you just do:
>
>int error = 0;
>
>stuff_call_here(var1, var2, &error);
>
>if (error != 0) {
>        ..
>}
>
>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.

>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.

-slow

-- 
Ralph Boehme, Samba Team                https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46



More information about the samba-technical mailing list