From mboxrd@z Thu Jan 1 00:00:00 1970 From: Serhey Popovich Subject: Re: [PATCH iproute2 1/3] iplink: Improve index parameter handling Date: Mon, 18 Dec 2017 23:37:09 +0200 Message-ID: <18e98fc6-4145-0bb9-143d-d4305d22bdc8@gmail.com> References: <1513623248-7689-1-git-send-email-serhe.popovych@gmail.com> <1513623248-7689-2-git-send-email-serhe.popovych@gmail.com> <20171218112359.1ef8db23@xeon-e3> <7f83992a-90e0-4f15-2be4-7348a6742e6c@gmail.com> <20171218132231.6dcf7b54@xeon-e3> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="JwrlSs9w9gaBb8lENris77V7FKosUbTa3" Cc: netdev@vger.kernel.org To: Stephen Hemminger Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:46663 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934193AbdLRVhQ (ORCPT ); Mon, 18 Dec 2017 16:37:16 -0500 Received: by mail-wm0-f65.google.com with SMTP id r78so414148wme.5 for ; Mon, 18 Dec 2017 13:37:15 -0800 (PST) In-Reply-To: <20171218132231.6dcf7b54@xeon-e3> Sender: netdev-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --JwrlSs9w9gaBb8lENris77V7FKosUbTa3 Content-Type: multipart/mixed; boundary="DKnLctmnN3O90jbwLsmqdBjcubO0aPEXj"; protected-headers="v1" From: Serhey Popovich To: Stephen Hemminger Cc: netdev@vger.kernel.org Message-ID: <18e98fc6-4145-0bb9-143d-d4305d22bdc8@gmail.com> Subject: Re: [PATCH iproute2 1/3] iplink: Improve index parameter handling References: <1513623248-7689-1-git-send-email-serhe.popovych@gmail.com> <1513623248-7689-2-git-send-email-serhe.popovych@gmail.com> <20171218112359.1ef8db23@xeon-e3> <7f83992a-90e0-4f15-2be4-7348a6742e6c@gmail.com> <20171218132231.6dcf7b54@xeon-e3> In-Reply-To: <20171218132231.6dcf7b54@xeon-e3> --DKnLctmnN3O90jbwLsmqdBjcubO0aPEXj Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Stephen Hemminger wrote: > On Mon, 18 Dec 2017 23:02:07 +0200 > Serhey Popovich wrote: >=20 >> Stephen Hemminger wrote: >>> On Mon, 18 Dec 2017 20:54:06 +0200 >>> Serhey Popovych wrote: >>> =20 >>>> diff --git a/ip/iplink.c b/ip/iplink.c >>>> index 1e685cc..4f9c169 100644 >>>> --- a/ip/iplink.c >>>> +++ b/ip/iplink.c >>>> @@ -586,8 +586,10 @@ int iplink_parse(int argc, char **argv, struct = iplink_req *req, >>>> *name =3D *argv; >>>> } else if (strcmp(*argv, "index") =3D=3D 0) { >>>> NEXT_ARG(); >>>> + if (*index) >>>> + duparg("index", *argv); >>>> *index =3D atoi(*argv); >>>> - if (*index < 0) >>>> + if (*index <=3D 0) =20 >>> >>> Why not use strtoul instead of atoi? =20 >> Do not see reason for strtoul() instead atoi(): >> >> 1) main arg: indexes in kernel represented as "int", which is >> signed. <=3D 0 values are reserved for various special purposes >> (see net/core/fib_rules.c on how device matching implemented). >> >> Configuring network device manually with index <=3D 0 is not corr= ect >> (however possible). Kernel itself never chooses ifindex <=3D 0. >> >> Having unsigned int > 0x7fffffff actually means index <=3D 0. >> >> 2) this is not single place in iproute2 where it is used: not >> going to remove last user. >> >> 3) make changes clear and transparent for review. >=20 > I would rather all of iproute2 correctly handles unsigned values. > Too much code is old K&R style C "the world is an int" and "who needs > to check for negative". You are right :(. I'm just trying to improve things a bit. >=20 > There already is get_unsigned() in iproute2 util functions. This is good one based on strtoul(). But do we want to submit say index =3D (unsigned int)2147483648(0x7fffffff) to the kernel that is illegal from it's perspective? Or do you mean I can prepare treewide change to replace atoi() with get_unsigned()/get_integer() where appropriate? We already check if (*index < 0) since commit 3c682146aeff (iplink: forbid negative ifindex and modifying ifindex), and I just put index =3D=3D 0 in the same range of invalid indexes. > Why not use that? >=20 --DKnLctmnN3O90jbwLsmqdBjcubO0aPEXj-- --JwrlSs9w9gaBb8lENris77V7FKosUbTa3 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQEcBAEBAgAGBQJaODUJAAoJEBTawMmQ61bBUHgH/30PpNKOp6KsyUQJnhAhtjQT WRnaDJugUJpb1vcHRERVsLZvqgZiTWtDdz1CNJZfNx+BD+FPLE0scbxmRVuPWpGD Ybf1H76uDW953KCUwlc5ZNDa2dReXFAiTNVL6OLO+61fCgTxc7/CdnXq5undimy6 24FEgHZVYyVIJaoyXT71y62jQbL0d293jzhKDJuahyIkvcCA9ZILvC83K0reAnLi 75S9tZhQsMKRC6NlSz6LASFz8FRPpuFVZfvMwV3sIJM5fbbBDKdcveOkHXHAQHPs ZNibbyyCKH4KHzWebQ7w4Wj2HoeMZgvwkoiRK671APHrMXA93II/tVruwwxIS9Q= =avEI -----END PGP SIGNATURE----- --JwrlSs9w9gaBb8lENris77V7FKosUbTa3--