[PATCH] Streamline messaging and serverid handling

[Ralph Boehme]

On Mon, Oct 19, 2015 at 01:02:10PM +0300, Uri Simchoni wrote:
> 
> 
> On 10/19/2015 11:41 AM, Ralph Boehme wrote:
> 
> >-	unique = strtoull(buf, &endptr, 10);
> >-	if ((unique == 0) && (errno == EINVAL)) {
> >-		return EINVAL;
> >-	}
> >-	if ((unique == ULLONG_MAX) && (errno == ERANGE)) {
> >-		return ERANGE;
> >-	}
> >-	if (endptr[0] != '\n') {
> >+	ret = sscanf(buf, SCNu64, &unique);
> >+	if ((ret != 1) || (unique == 0)) {
>
> I don't think sscanf is as robust as all the checks around
> strtoull. If I read the standard correctly (well, some draft from
> 2007), passing too many digits to scanf leads to undefined
> behavior. I also just tested it on FC22 - giving an out-of-range
> number returns 1 which is not the desired behavior.

Good catch! It seems glibc scanf returns ERANGE in some situations,
but it's reported to not catch all. uint64 seems to work, but you're
right, we can't rely on that. It's also not portable, as it's not in
the standard.

For reference, this works on Linux:

#include <stdio.h>
#include <stdint.h>
#include <inttypes.h>
#include <errno.h>

int main(int argc, char **argv)
{
        if (argc != 2) {
                printf("usage: %s NUMBER\n", argv[0]);
                return 1;
        }

        uint64_t unique;
        int result;

        result = sscanf(argv[1], "%" SCNu64, &unique);
        if (result != 1) {
                perror("sscanf");
                return 2;
        }
        if (errno == ERANGE) {
                perror("sscanf ERANGE");
                return 3;
        }

        printf("unique: %" PRIu64 "\n", unique);

        return 0;
}

$ ./test 18446744073709551615
unique: 18446744073709551615

$ ./test 18446744073709551616
sscanf ERANGE: Numerical result out of range

Not reliable, not portable, so we have to stick with casting to/from ull.

> What's more important IMHO is that such subtleties should not be
> part of application code because we don't want to be language
> lawyers to figure out the details - if it's important for the
> conversion to be robust then there should be a library function that
> does it, much like strlcpy() for strings. That function should use
> strtoull because strtoull is checked during configure and replaced
> by libreplace if necessary, and also does not include undefined
> behavior.
> 
> Where do you suppose we should put such a wrapper function?

lib/util/util_str.c. It already has a conv_str_u64().

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de