> -----Original Message----- > From: Stephen Hemminger [mailto:[email protected]] > Sent: Thursday, December 22, 2016 8:59 PM > To: Nogah Frankel <[email protected]> > Cc: [email protected]; [email protected]; [email protected]; > Or > Gerlitz <[email protected]>; Jiri Pirko <[email protected]>; Elad Raz > <[email protected]>; Yotam Gigi <[email protected]>; Ido Schimmel > <[email protected]> > Subject: Re: [PATCH iproute2 v3 2/4] ifstat: Add extended statistics to ifstat > > On Thu, 22 Dec 2016 18:23:13 +0200 > Nogah Frankel <[email protected]> wrote: > On Thu, 22 Dec 2016 18:23:13 +0200 > Nogah Frankel <[email protected]> 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.
