From owner-freebsd-current@FreeBSD.ORG Wed Dec 12 21:26:26 2012 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 90DD52EF; Wed, 12 Dec 2012 21:26:26 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id 26A2D8FC0A; Wed, 12 Dec 2012 21:26:26 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 84680358C2A; Wed, 12 Dec 2012 22:26:25 +0100 (CET) Received: by snail.stack.nl (Postfix, from userid 1677) id 6E6DB2848C; Wed, 12 Dec 2012 22:26:25 +0100 (CET) Date: Wed, 12 Dec 2012 22:26:25 +0100 From: Jilles Tjoelker To: Gleb Smirnoff Subject: Re: rev 244030 route command is not working Message-ID: <20121212212625.GA48633@stack.nl> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121211120703.GI48639@glebius.int.ru> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Artyom Mirgorodskiy , freebsd-current@FreeBSD.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Dec 2012 21:26:26 -0000 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