[PATCH] Fix string to integer conversion

[swen]

Hi Ralph

I just wish you would have come up with your thoughts/ideas
when the discussion was on 3-4 weeks ago :-)

On Mon, 2019-02-25 at 18:56 +0100, Ralph Böhme via samba-technical
wrote:
> Hi Swen,
> 
> On Mon, Feb 25, 2019 at 01:00:34PM +0000, Swen Schillig wrote:
> 
> 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;
> +               }
> 
What about if something like this is added ?

--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -64,6 +64,13 @@ strtoul_err(const char *nptr, char **endptr, int base, int *err)
                *endptr = tmp_endptr;
        }
 
+       if (nptr == tmp_endptr) {
+               /* we got an invalid number-string resulting in no conversion */
+               *err = EINVAL;
+               errno = save_errno;
+               return val;
+       }
+
        if (errno != 0) {
                *err = errno;
                errno = saved_errno;

This will catch the non-conversion case and leaves everything else as it is ?

> The strtoul API is broken and we shouldn't add it to Samba. 
Well it is there already :-)

> I know it's a pita 
> to change the many callers once again, I'm willing to lend a helping
> hand here.
I don't mind the work but I would appreciate if we could find a
solution which addresses the issue and to which at least 2 team-members 
could agree to :-)
....oh and if possible without re-inventing the wheel.

Please let me know what you think about the above suggestion ?
It would be a rather small change and deals with the issue you brought
up.

Cheers Swen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190226/0ec6d8d2/signature.sig>