[PATCH] Fix string to integer conversion
Ralph Böhme
slow at samba.org
Tue Feb 26 10:15:05 UTC 2019
On Tue, Feb 26, 2019 at 09:22:07AM +0100, swen wrote:
>Hi Ralph
>
>I just wish you would have come up with your thoughts/ideas
>when the discussion was on 3-4 weeks ago :-)
yeah, sorry, been really busy so I wasn't able to keep up with some stuff.
>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 ?
Then I guess we also need
+ if (nptr[0] == '\0') {
+ /* we got an empty string */
+ *err = EINVAL;
+ errno = save_errno;
+ return val;
+ }
+
before the call to strtoul().
Then lets remove the out-parameter uglyness and we're going somewhere:
bool tstrtoull(const char *nptr, char **endptr, int base, unsigned long long int *val);
:)
Let's see what Jeremy has to say on it, I'll give him a chance to convince me
that strtoull_err() in the current form is really a good idea.
I imagine one previous argument has been, that a strict implementation of a new
conversion function, be it strtoull_err() or tstrtoull(), may not be suitable to
replace the existing call to strtoull() in all places due to the behaviour
change?
Btw, there'a bug in strtoull_err(), fixup attached.
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG-Fingerprint FAE2C6088A24252051C559E4AA1E9B7126399E46
-------------- next part --------------
From 30162f931668894e883d4f125fb34cb33f86e3c7 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 26 Feb 2019 10:55:03 +0100
Subject: [PATCH] FIXUP
---
lib/util/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/util/util.c b/lib/util/util.c
index 428bad7d285..ce91e8aa44b 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -91,7 +91,7 @@ strtoull_err(const char *nptr, char **endptr, int base, int *err)
errno = 0;
*err = 0;
- val = strtoull(nptr, endptr, base);
+ val = strtoull(nptr, &tmp_endptr, base);
if (endptr != NULL) {
*endptr = tmp_endptr;
--
2.17.2
More information about the samba-technical
mailing list