Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 12 Dec 2012 22:26:25 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
To:        Gleb Smirnoff <glebius@FreeBSD.org>
Cc:        Artyom Mirgorodskiy <artyom@ijminteractive.net>, freebsd-current@FreeBSD.org
Subject:   Re: rev 244030 route command is not working
Message-ID:  <20121212212625.GA48633@stack.nl>
In-Reply-To: <20121211120703.GI48639@glebius.int.ru>
References:  <2452291.zQQ4fSp1fM@home.alkar.net> <20121211071334.GS48639@glebius.int.ru> <1740464.YojJrNfMlV@notebook.alkar.net> <13928549.1YcT8qshV5@notebook.alkar.net> <20121211120703.GI48639@glebius.int.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Dec 11, 2012 at 04:07:03PM +0400, Gleb Smirnoff wrote:
> On Tue, Dec 11, 2012 at 12:21:20PM +0200, Artyom Mirgorodskiy wrote:
> A> Gleb, when I reset errno at the begin of fiboptlist_csv()
> A> everything work as expected.

> Artyom,

> can you please test attached patch?

> Index: route.c
> ===================================================================
> --- route.c	(revision 244082)
> +++ route.c	(working copy)
> @@ -271,8 +271,7 @@
>  		case 0:
>  		case 1:
>  			fib[i] = strtol(token, &endptr, 0);
> -			if (*endptr != '\0' || (fib[i] == 0 &&
> -			    (errno == EINVAL || errno == ERANGE)))
> +			if (*endptr != '\0')
>  				error = 1;
>  			break;
>  		default:
> @@ -336,8 +335,7 @@
>  				goto fiboptlist_csv_ret;
>  		} else {
>  			fib = strtol(token, &endptr, 0);
> -			if (*endptr != '\0' || (fib == 0 &&
> -			    (errno == EINVAL || errno == ERANGE))) {
> +			if (*endptr != '\0') {
>  				error = 1;
>  				goto fiboptlist_csv_ret;
>  			}

This patch will avoid erroneously failing but will let through certain
invalid strings. The empty string will silently be treated as 0 and
values outside the range [LONG_MIN, LONG_MAX] will be clamped silently
(the latter was already broken in most cases because [ERANGE] happens
only with a return value of LONG_MIN or LONG_MAX).

Other invalid strings that were and are let through (with unexpected
results) are ones containing a number that fits in a long but not in an
int.

To fix the range detection, errno should be set to 0 before the call and
checked afterwards; in a library function (this is an application),
errno should be saved and restored around that to avoid setting errno to
0 as a side effect of the function. The empty string needs a specific
check.

I don't insist on this being fixed but it shows that strtol() is too
hard to use correctly. The non-standard strtonum() looks easier but has
other problems (such as returning an English string in an API that
should be language-neutral).

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121212212625.GA48633>