> -----Original Message----- > From: Stephen Hemminger [mailto:step...@networkplumber.org] > Sent: Thursday, December 22, 2016 8:59 PM > To: Nogah Frankel <nog...@mellanox.com> > Cc: netdev@vger.kernel.org; ro...@cumulusnetworks.com; roszenr...@gmail.com; > Or > Gerlitz <ogerl...@mellanox.com>; Jiri Pirko <j...@mellanox.com>; Elad Raz > <el...@mellanox.com>; Yotam Gigi <yot...@mellanox.com>; Ido Schimmel > <ido...@mellanox.com> > Subject: Re: [PATCH iproute2 v3 2/4] ifstat: Add extended statistics to ifstat > > On Thu, 22 Dec 2016 18:23:13 +0200 > Nogah Frankel <nog...@mellanox.com> wrote: > On Thu, 22 Dec 2016 18:23:13 +0200 > Nogah Frankel <nog...@mellanox.com> wrote: > > > } > > @@ -691,18 +804,22 @@ static const struct option longopts[] = { > > { "interval", 1, 0, 't' }, > > { "version", 0, 0, 'V' }, > > { "zeros", 0, 0, 'z' }, > > + { "extended", 1, 0, 'x'}, > > { 0 } > > }; > > > > + > > int main(int argc, char *argv[]) > > You let extra whitespace changes creep in. > > > > + case 'x': > > + is_extended = true; > > + memset(stats_type, 0, 64); > > + strncpy(stats_type, optarg, 63); > > + break; > > This seems like doing this either the paranoid or hard way. > Why not: > const char *stats_type = NULL; > ... > > case 'x': > stats_type = optarg; > break; > ... > if (stats_type) > snprintf(hist_name, sizeof(hist_name), > "%s/.%s_ifstat.u%d", P_tmpdir, stats_type, > getuid()); > else > snprintf(hist_name, sizeof(hist_name), > "%s/.ifstat.u%d", P_tmpdir, getuid()); > > > Since: > 1) optarg points to area in argv that is persistent (avoid copy) > 2) don't need is_extended flag value then > > Please cleanup and resubmit. > >
I will. Thank you.
