Re: [PATCH iproute2 v3 2/4] ifstat: Add extended statistics to ifstat

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 22 Dec 2016 18:23:13 +0200
Nogah Frankel <nogahf@mellanox.com> wrote:
On Thu, 22 Dec 2016 18:23:13 +0200
Nogah Frankel <nogahf@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.