[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6DD00483C5@AcuExch.aculab.com>
Date: Tue, 1 Aug 2017 15:12:11 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Stephen Hemminger' <stephen@...workplumber.org>,
Amritha Nambiar <amritha.nambiar@...el.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"alexander.h.duyck@...el.com" <alexander.h.duyck@...el.com>,
"kiran.patil@...el.com" <kiran.patil@...el.com>,
"sridhar.samudrala@...el.com" <sridhar.samudrala@...el.com>,
"mitch.a.williams@...el.com" <mitch.a.williams@...el.com>,
"alexander.duyck@...il.com" <alexander.duyck@...il.com>,
"neerav.parikh@...el.com" <neerav.parikh@...el.com>,
"carolyn.wyborny@...el.com" <carolyn.wyborny@...el.com>,
"jeffrey.t.kirsher@...el.com" <jeffrey.t.kirsher@...el.com>
Subject: RE: [PATCH RFC, iproute2] tc/mirred: Extend the mirred/redirect
action to accept additional traffic class parameter
From: Stephen Hemminger
> Sent: 01 August 2017 04:52
> On Mon, 31 Jul 2017 17:40:50 -0700
> Amritha Nambiar <amritha.nambiar@...el.com> wrote:
> The concept is fine, bu t the code looks different than the rest which
> is never a good sign.
>
>
> > + if ((argc > 0) && (matches(*argv, "tc") == 0)) {
>
> Extra () are unnecessary in compound conditional.
>
> > + tc = atoi(*argv);
>
> Prefer using strtoul since it has better error handling than atoi()
>
> > + argc--;
> > + argv++;
> > + }
>
>
> Use NEXT_ARG() construct like rest of the code.
Why bother faffing about with argc at all?
The argument list terminates when *argv == NULL.
David
Powered by blists - more mailing lists