DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Richardson, Bruce" <bruce.richardson@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/3] stringfns: remove rte_snprintf
Date: Tue, 24 Jun 2014 18:00:20 +0000	[thread overview]
Message-ID: <59AF69C657FD0841A61C55336867B5B02CEE345B@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <20140624105427.166c21bf@nehalam.linuxnetplumber.net>

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Tuesday, June 24, 2014 10:54 AM
> To: Richardson, Bruce
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/3] stringfns: remove rte_snprintf
> 
> On Tue, 24 Jun 2014 17:39:52 +0000
> "Richardson, Bruce" <bruce.richardson@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> Hemminger
> > > Sent: Tuesday, June 24, 2014 9:03 AM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 1/3] stringfns: remove rte_snprintf
> > >
> > > The function rte_snprintf serves no useful purpose. It is the
> > > same as snprintf() for all valid inputs. Just remove it and
> > > replace all uses in current code.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > >
> >
> > NAK to this as is.
> > Approve of replacing all instance of the rte_snprintf function with the standard
> version in all our code and libraries.
> > However, rather than just removing the function completely, I think we should
> just flag the function as deprecated initially, and then later on look to remove it
> completely.
> >
> > /Bruce
> 
> I want to get it out now rather than some 2 year life cycle.
> The issue was discussed and marking it as deprecated breaks the build.
> Alternate is removing all instances and adding:
> 
> #define rte_snprintf	snprintf
> 
> in header file for user compatiablity.

We can remove it from all our apps and then mark as deprecated and all our code would still build.
An interesting point I'd never thought of is, is it right for us to force user apps to treat all warnings as errors? Perhaps we should also consider removing -Werror from the CFLAGS when using rte.extapp.mk.

  reply	other threads:[~2014-06-24 18:01 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-24 16:02 Stephen Hemminger
2014-06-24 16:03 ` [dpdk-dev] [PATCH 2/3] test: remove no longer valid tests Stephen Hemminger
2014-06-24 16:05 ` [dpdk-dev] [PATCH 3/3] fix incorrect snprintf usage Stephen Hemminger
2014-06-24 17:39 ` [dpdk-dev] [PATCH 1/3] stringfns: remove rte_snprintf Richardson, Bruce
2014-06-24 17:54   ` Stephen Hemminger
2014-06-24 18:00     ` Richardson, Bruce [this message]
2014-06-25  7:55       ` Olivier MATZ
2014-06-25  8:33   ` Thomas Monjalon
2014-06-26 15:09     ` Richardson, Bruce
2014-06-26 16:20       ` Aaron Campbell
2014-06-27  0:37         ` Thomas Monjalon

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=59AF69C657FD0841A61C55336867B5B02CEE345B@IRSMSX103.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /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).