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>