[PATCH] Fix string to integer conversion

[Ralph Böhme]

Hi Swen,

On Mon, Feb 25, 2019 at 01:00:34PM +0000, Swen Schillig wrote:
>>The patchset introduces an additional error check in the callers in some 
>>places where only the return error is checked, causing an error log message or 
>>even a failure in the caller.  The problem is, according to POSIX/SUS, strtoul 
>>and friends are not *required* to set errno to EINVAL "in case no conversion 
>>was >performed". The implementation of strtoul[l]_err() doesn't set it, it 
>>just returns the errno from strtoul.
>
>The POSIX standard says that errno doesn't need to be set if an error occured 
>but we can be sure that IF IT WAS SET then we do have an error...and nothing 
>else is implemented with the new functions.  We do NOT flag an error if there 
>wasn't one.

guess I didn't express myself right, but that's not why I was trying to say.

The problem is that the new function and the use of the new function in a bunch 
of places suggests that you can *rely* on the error out variable being for 
strings like "" or "foo".

Eg in one place your patch adds this:

--- a/ctdb/tools/ctdb.c
+++ b/ctdb/tools/ctdb.c
@@ -544,7 +545,10 @@ static bool db_exists(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb,
        }

-               id = strtoul(db_arg, NULL, 0);
+               id = strtoul_err(db_arg, NULL, 0, &ret);
+               if (ret != 0) {
+                       return false;
+               }

This now requires knowledge of noth strtoul() and strtoul_err() to understand 
under which conditions ret will be != 0.

The future Samba hacker looking for examples how to use the function might come 
across this fragment and then jump to the wrong conclusions.

The strtoul API is broken and we shouldn't add it to Samba. I know it's a pita 
to change the many callers once again, I'm willing to lend a helping hand here.

-slow

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