From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v1 2/3] net/hyperv: implement core
 functionality
Date: Tue, 19 Dec 2017 07:31:30 -0800
Message-ID: <20171219073130.3b9662b0@xeon-e3>
References: <20171124172132.GW4062@6wind.com>
 <20171218162443.12971-1-adrien.mazarguil@6wind.com>
 <20171218162443.12971-3-adrien.mazarguil@6wind.com>
 <20171218103412.342adcbc@xeon-e3> <20171218202341.GF4062@6wind.com>
 <20171219095326.GA7384@bricha3-MOBL3.ger.corp.intel.com>
 <20171219101538.GC5377@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Bruce Richardson <bruce.richardson@intel.com>, Ferruh Yigit
 <ferruh.yigit@intel.com>, dev@dpdk.org
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pf0-f196.google.com (mail-pf0-f196.google.com
 [209.85.192.196]) by dpdk.org (Postfix) with ESMTP id 8C182271
 for <dev@dpdk.org>; Tue, 19 Dec 2017 16:31:33 +0100 (CET)
Received: by mail-pf0-f196.google.com with SMTP id d23so11243838pfe.9
 for <dev@dpdk.org>; Tue, 19 Dec 2017 07:31:33 -0800 (PST)
In-Reply-To: <20171219101538.GC5377@6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Tue, 19 Dec 2017 11:15:38 +0100
Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:

> On Tue, Dec 19, 2017 at 09:53:27AM +0000, Bruce Richardson wrote:
> > On Mon, Dec 18, 2017 at 09:23:41PM +0100, Adrien Mazarguil wrote:  
> > > On Mon, Dec 18, 2017 at 10:34:12AM -0800, Stephen Hemminger wrote:  
> > > > On Mon, 18 Dec 2017 17:46:23 +0100
> > > > Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
> > > >   
> > <snip>  
> > > > > +static int
> > > > > +hyperv_iface_is_netvsc(const struct if_nameindex *iface)
> > > > > +{
> > > > > +	static const char temp[] = "/sys/class/net/%s/device/class_id";
> > > > > +	char path[snprintf(NULL, 0, temp, iface->if_name) + 1];  
> > > > 
> > > > Doing this snprintf is gross. Either use PATH_MAX or asprintf  
> > > 
> > > I don't think allocating more stack space than necessary or on the heap with
> > > a possible allocation failure to deal with is any better, sorry.
> > > 
> > > Prove this snprintf() call can fail and you'll have a point.
> > >   
> > While I get your point, I'd tend to go with Stephen's view on this that
> > it's looking a bit "gross". What's the problem with allocating a bit
> > more stack space for it?  
> 
> Well, apart from making a stand, none really. Too "unusual" perhaps, but I
> don't think "gross" is a valid argument to reject a perfectly valid piece of
> code that doesn't rely on obscure knowledge nor weird side effects.
> 
> I'll update this in v2 to make it look more acceptable in any case.
> 

In this particular case, you can easily show that the maximum length of
the string would be less than the format plus maximum length of interface
name.

Why not:
	char path[sizeof(temp) + IFNAMSIZ];
which keeps the flexibility but also can be evaluated at compile time.


Upleveling. You need to understand that open source software is a collabrative
effort. And like doing improvisational theatre, the best answer to any
feedback is yes unless there is a technical reason otherwise.