netdev - RE: [PATCH iproute2 v3 2/4] ifstat: Add extended statistics to ifstat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <VI1PR05MB1902B99C24BF14CAC3DA6B5BAC970@VI1PR05MB1902.eurprd05.prod.outlook.com>
Date:   Sun, 25 Dec 2016 10:25:18 +0000
From:   Nogah Frankel <nogahf@...lanox.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "roopa@...ulusnetworks.com" <roopa@...ulusnetworks.com>,
        "roszenrami@...il.com" <roszenrami@...il.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Jiri Pirko <jiri@...lanox.com>, Elad Raz <eladr@...lanox.com>,
        Yotam Gigi <yotamg@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>
Subject: RE: [PATCH iproute2 v3 2/4] ifstat: Add extended statistics to ifstat

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@...workplumber.org]
> Sent: Thursday, December 22, 2016 8:59 PM
> To: Nogah Frankel <nogahf@...lanox.com>
> Cc: netdev@...r.kernel.org; roopa@...ulusnetworks.com; roszenrami@...il.com; Or
> Gerlitz <ogerlitz@...lanox.com>; Jiri Pirko <jiri@...lanox.com>; Elad Raz
> <eladr@...lanox.com>; Yotam Gigi <yotamg@...lanox.com>; Ido Schimmel
> <idosch@...lanox.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 <nogahf@...lanox.com> wrote:
> On Thu, 22 Dec 2016 18:23:13 +0200
> Nogah Frankel <nogahf@...lanox.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.