From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH iproute2 1/3] iplink: Improve index parameter handling Date: Mon, 18 Dec 2017 13:22:31 -0800 Message-ID: <20171218132231.6dcf7b54@xeon-e3> 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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; boundary="Sig_/uz88rqgiDAn/Lo9LOv4EepU"; protocol="application/pgp-signature" Cc: netdev@vger.kernel.org To: Serhey Popovich Return-path: Received: from mail-pg0-f65.google.com ([74.125.83.65]:42289 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934355AbdLRVWf (ORCPT ); Mon, 18 Dec 2017 16:22:35 -0500 Received: by mail-pg0-f65.google.com with SMTP id q67so434888pga.9 for ; Mon, 18 Dec 2017 13:22:35 -0800 (PST) In-Reply-To: <7f83992a-90e0-4f15-2be4-7348a6742e6c@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: --Sig_/uz88rqgiDAn/Lo9LOv4EepU Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Mon, 18 Dec 2017 23:02:07 +0200 Serhey Popovich wrote: > 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 ip= link_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 > >=20 > > Why not use strtoul instead of atoi? =20 > Do not see reason for strtoul() instead atoi(): >=20 > 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). >=20 > Configuring network device manually with index <=3D 0 is not correct > (however possible). Kernel itself never chooses ifindex <=3D 0. >=20 > Having unsigned int > 0x7fffffff actually means index <=3D 0. >=20 > 2) this is not single place in iproute2 where it is used: not > going to remove last user. >=20 > 3) make changes clear and transparent for review. 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". There already is get_unsigned() in iproute2 util functions. Why not use that? --Sig_/uz88rqgiDAn/Lo9LOv4EepU Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEn2/DRbBb5+dmuDyPgKd/YJXN5H4FAlo4MZcACgkQgKd/YJXN 5H47FhAAkq5Mm9oR7j+AXU6IYekPlxPETSx88PACxZhIx//yMJEYePblw+pwIqVX KP5gkPWrzQ6p3zY10mTHcHHloohay0p3w16XkX/RTnpIM2i4Fzm7N9OrbJbgUw93 816PiS1Nt/+jnRieOZEu7xT9yt1RMrqdzXMJzCoNUCY2nLmByJw2FkzoN/hfCOWx L9a27inkanp8UomQg3qPPe9fyUXTV+MEV6KnHNPSZ9VytSUuU5vbhFufh/QoYQs0 l58Vwl0m9BPApIsq9xY99X6EwEKahfSHVjHyJX/t0LNG9fPDySebYps96ggnHSZZ 9ghNxs3Vel30uSnHODrLCoxW7yEeCXdHAVZIsG6DL69dCvC98qSpcc1GCrrAorsj DCGSxH9AjKw24FNM33ZfUen6o9z6nD/qRCE5HqhLqzsEUDlkfhMIJYub7h81f5W0 jnwvol3g7/92J0TYMo5JDyCUFBl5zaxQuvzD3xptd/NH14CVCqzvBPXh5bZX1Bt5 YWyOOlykIAlSHKSU99B6vZxGDyMF5LF6gXS8hNmNMpekLCj+oIQBICuoX0Lf4qTw GjVLUJuivVAktuMTha4Hk63U056jfjGhrTFKW4QAI1CkPLe7xrnh3CYpcnNJCiPt UgqLYFjPCOVcH49st4OfyQfhJj2Lldh9lqu5h+10MgrlnF8y65c= =ZXqX -----END PGP SIGNATURE----- --Sig_/uz88rqgiDAn/Lo9LOv4EepU--