DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier MATZ <olivier.matz@6wind.com>
To: "Richardson, Bruce" <bruce.richardson@intel.com>,
	 Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/3] stringfns: remove rte_snprintf
Date: Wed, 25 Jun 2014 09:55:47 +0200
Message-ID: <53AA8083.8000900@6wind.com> (raw)
In-Reply-To: <59AF69C657FD0841A61C55336867B5B02CEE345B@IRSMSX103.ger.corp.intel.com>

Hi Bruce,

On 06/24/2014 08:00 PM, Richardson, Bruce wrote:
>> 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.

I don't understand why removing a function like rte_snprintf()
in a new version of DPDK would be a problem. Yes, it would break
the build of external applications, but that's the best way to fix
the problem in the external code. Adding a compatibility layer
just delays the issue. Maybe having a line about this in a "release
note" or a "porting guide" would be enough? It could even references
the commit id, showing how to solve the issue.

About the -Werror flag, I would say that removing it is not a good
idea. From my experience, many issues are pointed out by warnings,
and forcing the compiler to stop on warning helps to have better
code quality.

Regards,
Olivier

  reply	other threads:[~2014-06-25  7:55 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
2014-06-25  7:55       ` Olivier MATZ [this message]
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=53AA8083.8000900@6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git