DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality
Date: Tue, 19 Dec 2017 07:31:30 -0800	[thread overview]
Message-ID: <20171219073130.3b9662b0@xeon-e3> (raw)
In-Reply-To: <20171219101538.GC5377@6wind.com>

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.

  reply	other threads:[~2017-12-19 15:31 UTC|newest]

Thread overview: 112+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20171124160801.GU4062@6wind.com>
     [not found] ` <20171124164812.GV4062@6wind.com>
2017-11-24 17:21   ` [dpdk-dev] [RFC] Introduce virtual PMD for Hyper-V/Azure platforms Adrien Mazarguil
2017-12-18 16:46     ` [dpdk-dev] [PATCH v1 0/3] " Adrien Mazarguil
2017-12-18 16:46       ` [dpdk-dev] [PATCH v1 1/3] net/hyperv: introduce MS Hyper-V platform driver Adrien Mazarguil
2017-12-18 18:28         ` Stephen Hemminger
2017-12-18 19:54           ` Thomas Monjalon
2017-12-18 21:17             ` Stephen Hemminger
2017-12-19 10:01               ` Adrien Mazarguil
2017-12-19 11:15                 ` Thomas Monjalon
2017-12-19 13:13                   ` Adrien Mazarguil
2017-12-18 16:46       ` [dpdk-dev] [PATCH v1 2/3] net/hyperv: implement core functionality Adrien Mazarguil
2017-12-18 17:04         ` Wiles, Keith
2017-12-18 17:59           ` Adrien Mazarguil
2017-12-18 18:43             ` Wiles, Keith
2017-12-19  8:25               ` Nelio Laranjeiro
2017-12-18 18:26         ` Stephen Hemminger
2017-12-18 20:21           ` Adrien Mazarguil
2017-12-18 21:03             ` Thomas Monjalon
2017-12-18 21:19               ` Stephen Hemminger
2017-12-18 18:34         ` Stephen Hemminger
2017-12-18 20:23           ` Adrien Mazarguil
2017-12-19  9:53             ` Bruce Richardson
2017-12-19 10:15               ` Adrien Mazarguil
2017-12-19 15:31                 ` Stephen Hemminger [this message]
2017-12-18 23:59         ` Stephen Hemminger
2017-12-19 10:01           ` Adrien Mazarguil
2017-12-19 15:37             ` Stephen Hemminger
2017-12-19  1:54         ` Ferruh Yigit
2017-12-19 15:06           ` Adrien Mazarguil
2017-12-19 20:44             ` Ferruh Yigit
2017-12-20 14:13               ` Thomas Monjalon
2017-12-21 16:19               ` Adrien Mazarguil
2017-12-18 16:46       ` [dpdk-dev] [PATCH v1 3/3] net/hyperv: add "force" parameter Adrien Mazarguil
2017-12-18 18:23       ` [dpdk-dev] [PATCH v1 0/3] Introduce virtual PMD for Hyper-V/Azure platforms Stephen Hemminger
2017-12-18 20:13         ` Thomas Monjalon
2017-12-19  0:40           ` Stephen Hemminger
2017-12-18 20:21         ` Adrien Mazarguil
2017-12-22 18:01       ` [dpdk-dev] [PATCH v2 0/5] " Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 1/5] net/failsafe: fix invalid free Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 2/5] net/failsafe: add "fd" parameter Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 3/5] net/vdev_netvsc: introduce Hyper-V platform driver Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 4/5] net/vdev_netvsc: implement core functionality Adrien Mazarguil
2017-12-22 18:01         ` [dpdk-dev] [PATCH v2 5/5] net/vdev_netvsc: add "force" parameter Adrien Mazarguil
2017-12-23  2:06         ` [dpdk-dev] [PATCH v2 0/5] Introduce virtual PMD for Hyper-V/Azure platforms Stephen Hemminger
2017-12-23 14:28           ` Thomas Monjalon
2018-01-09 14:47         ` [dpdk-dev] [PATCH v3 0/8] Introduce virtual driver " Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-16 10:24             ` Gaëtan Rivet
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-16 10:54             ` Gaëtan Rivet
2018-01-16 11:19               ` Gaëtan Rivet
2018-01-16 16:17                 ` Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 3/8] net/failsafe: support probed sub-devices getting Matan Azrad
2018-01-16 11:09             ` Gaëtan Rivet
2018-01-16 12:27               ` Matan Azrad
2018-01-16 14:40                 ` Gaëtan Rivet
2018-01-16 16:15                   ` Matan Azrad
2018-01-16 16:54                     ` Gaëtan Rivet
2018-01-16 17:20                       ` Matan Azrad
2018-01-16 22:31                         ` Gaëtan Rivet
2018-01-17  8:40                           ` Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-09 18:49             ` Stephen Hemminger
2018-01-10 15:02               ` Matan Azrad
2018-01-17 16:51                 ` Thomas Monjalon
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-09 18:51             ` Stephen Hemminger
2018-01-10 15:07               ` Matan Azrad
2018-01-10 16:43                 ` Stephen Hemminger
2018-01-11  9:00                   ` Matan Azrad
2018-01-17 16:59                     ` Thomas Monjalon
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-09 14:47           ` [dpdk-dev] [PATCH v3 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18  8:43           ` [dpdk-dev] [PATCH v4 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18  8:51               ` Gaëtan Rivet
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18  9:10               ` Gaëtan Rivet
2018-01-18  9:33                 ` Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 18:25               ` Stephen Hemminger
2018-01-18 18:28                 ` Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 18:26               ` Stephen Hemminger
2018-01-18 18:47                 ` Thomas Monjalon
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 18:27               ` Stephen Hemminger
2018-01-18 18:30                 ` Matan Azrad
2018-01-18  8:43             ` [dpdk-dev] [PATCH v4 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 10:01             ` [dpdk-dev] [PATCH v5 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 10:08                 ` Gaëtan Rivet
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 10:01               ` [dpdk-dev] [PATCH v5 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-18 13:51               ` [dpdk-dev] [PATCH v6 0/8] Introduce virtual driver for Hyper-V/Azure platforms Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 1/8] net/failsafe: fix invalid free Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 2/8] net/failsafe: add "fd" parameter Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 3/8] net/failsafe: add probed etherdev capture Matan Azrad
2018-01-18 22:34                   ` Thomas Monjalon
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 4/8] net/vdev_netvsc: introduce Hyper-V platform driver Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 5/8] net/vdev_netvsc: implement core functionality Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 6/8] net/vdev_netvsc: skip routed netvsc probing Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 7/8] net/vdev_netvsc: add "force" parameter Matan Azrad
2018-01-18 13:51                 ` [dpdk-dev] [PATCH v6 8/8] net/vdev_netvsc: add automatic probing Matan Azrad
2018-01-20  1:15                 ` [dpdk-dev] [PATCH v6 0/8] Introduce virtual driver for Hyper-V/Azure platforms Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171219073130.3b9662b0@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).