Re: [patch] give gpioctl.c some love

[Timo Buhrmester <fstd.lkml%gmail.com@localhost>]

> But while most of the changes are just objectionable, this part
> is just plain funny (and sad...)
> 
>   | +static unsigned
>   | +get_uint_or_die(const char *str, const char *name)
>   | +{
>   | +	int oerrno;
>   | +	char *ep;
>   | +
>   | +	long lval = strtol(str, &ep, 0);
>   | +	if (!*str || *ep)
>   | +		errx(EXIT_FAILURE, "invalid %s (not a number)", name);
>   | +
>   | +	oerrno = errno, errno = 0;
>   | +
>   | +	if ((errno == ERANGE && (lval == LONG_MAX || lval == LONG_MIN))
>   | +	    || lval < 0 || (unsigned long)lval > UINT_MAX)
>   | +		errx(EXIT_FAILURE, "%s out of range", name);
>   | +
>   | +	errno = oerrno;
>   | +
>   | +	return (unsigned)lval;
>   | +}
> 
> Clearly that was never really tested (or even read checked), was it?
Quite the opposite, it was, and in fact (as stated in the original mail) it fixes mistakenly accepting some negative values.
However it's not like I came up with that code, I just wrapped it in a function with the same semantics as in both occurences where the code appeared in-line, beforehands.
Again, because it was quite hard to read.
|-		lval = strtol(mask, &ep, 0);
|-		if (*mask == '\0' || *ep != '\0')
|-			errx(EXIT_FAILURE, "invalid mask (not a number)");
|-		if ((errno == ERANGE && (lval == LONG_MAX
|-		    || lval == LONG_MIN)) || (unsigned long)lval > UINT_MAX)
|-			errx(EXIT_FAILURE, "mask out of range");
|-		ga_mask = lval;
|-		if (flags != NULL) {
|-			lval = strtol(flags, &ep, 0);
|-			if (*flags == '\0' || *ep != '\0')
|-				errx(EXIT_FAILURE,
|-				    "invalid flag locator (not a number)");
|-			if ((errno == ERANGE && (lval == LONG_MAX
|-			    || lval == LONG_MIN))
|-			    || (unsigned long)lval > UINT_MAX)
|-				errx(EXIT_FAILURE, "flag locator out of range");
|-
|-			ga_flags = lval;
|-		}

I'm not sure I get what's either funny or sad about it, nor why you'd assume it wasn't even read-checked.