[PATCH] Fix string to integer conversion

[swen]

Hi Ralph

thanks for going through all this effort but ... I have a few comments

On Tue, 2019-02-26 at 13:55 +0100, Ralph Böhme wrote:
> > Yeah, I might be willing to cross that bridge. Let me go through
> > the 
> > changes in the caller. Stay tuned...
> 
> here's a branch with a few fixups:
> 
> <
> https://git.samba.org/?p=slow/samba.git;a=shortlog;h=refs/heads/strto_err
> >
> 
> We also need a doxygen comment on the new functions that explain
> possible 
> results and errors.
Ok, I will do that..

> 
> For now I think we should just go with the integeroverflow and signed
> detection.
Why not adding the last one detecting the no-conversion patch ?

Now a few comments regarding the FIXUPS.

general:
Why do split all initializations at declaration time into two hunks ?
I don't see a reason for that.

diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 4836697..b233c68 100644 (file)
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -1454,6 +1454,9 @@ static int messaging_dgm_read_unique(int fd,
uint64_t *punique)
        buf[rw_ret] = '\0';
 
        unique = strtoull_err(buf, &endptr, 10, &error);
+       if ((unique == 0) && (errno == EINVAL)) {
+               return EINVAL;
+       }
        if (error != 0) {
                return error;
        }
This is not needed. Exactly the same happens without this addition.


diff --git a/source3/utils/pdbedit.c b/source3/utils/pdbedit.c
index 51db671..c80d541 100644 (file)
--- a/source3/utils/pdbedit.c
+++ b/source3/utils/pdbedit.c
@@ -599,14 +599,13 @@ static int set_user_info(const char *username,
const char *fullname,
 
                if (strcmp(kickoff_time, "never") != 0) {
                        int error = 0;
-                       uint32_t num = strtoul_err(kickoff_time,
-                                                  &endptr,
-                                                  10,
-                                                  &error);
+                       uint32_t num;
 
+                       num = strtoul_err(kickoff_time, &endptr, 10,
&error);
                        if ((endptr == kickoff_time) ||
                            (endptr[0] != '\0') ||
-                           (error != 0)) {
+                           (error != 0))
+                       {
                                fprintf(stderr, "Failed to parse
kickoff time\n");
                                return -1;
                        }
I'm pretty sure our coding guide says somehting else.


Ok, I guess there's no poijhnt in commenting all of them.
At the end of the day, if that makes you feel better about it,
I won't object.

How do we proceed from here ?
I have the full patchset ready (incl. the no-conversion patch) but
without your fixups.
Since you have it all together, I guess there's no need for me to send
it again, right ?

So please let me know what's next.

Thanks for your support in advance, really appreciated !!!

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/171218ee/signature.sig>