[PATCH] Streamline messaging and serverid handling

[Uri Simchoni]

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.

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?

Thanks,
Uri