From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH v4 01/23] lib/librte_eal: import libbsd strlcpy Date: Thu, 17 May 2018 14:49:16 +0100 Message-ID: <20180517134916.GA22288@bricha3-MOBL.ger.corp.intel.com> References: <152627436523.53156.4398253089110011263.stgit@localhost.localdomain> <152627457252.53156.7037125685610031955.stgit@localhost.localdomain> <20180517103658.GA16984@bricha3-MOBL.ger.corp.intel.com> <78e21755-2ef2-8a72-18cd-40e23f0d342e@warmcat.com> <20180517125625.GA32@bricha3-MOBL.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org To: Andy Green Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 793BC7CBA for ; Thu, 17 May 2018 15:49:21 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, May 17, 2018 at 09:00:25PM +0800, Andy Green wrote: > > > On 05/17/2018 08:56 PM, Bruce Richardson wrote: > > On Thu, May 17, 2018 at 08:35:21PM +0800, Andy Green wrote: > > > > > > > > > On 05/17/2018 06:36 PM, Bruce Richardson wrote: > > > > On Mon, May 14, 2018 at 01:09:32PM +0800, Andy Green wrote: > > > > > Signed-off-by: Andy Green > > > > > --- > > > > > lib/librte_eal/common/eal_common_string_fns.c | 34 ++++++++++++++++++++++++ > > > > > lib/librte_eal/common/include/rte_string_fns.h | 7 +---- > > > > > 2 files changed, 36 insertions(+), 5 deletions(-) > > > > > > > > > > > > > While I'm aware this was suggested by other reviewers, I really don't feel > > > > that it is necessary to actually import the code. If libbsd is present on > > > > the system, we will use it directly. If libbsd is not present, the snprintf > > > > provides an acceptable fallback for strlcpy IMHO. Having the full function > > > > without good justification seems excessive. > > > > > > Well, as you can probably guess, I don't really mind either way. > > > > > > This also implies another patch to export rte_strlcpy since it's no longer > > > an inline in the headers this way. > > > > > > I removed these patches and rebuilt dpdk and then lagopus without it with > > > the idea of pasting the compile error. But I can't reproduce the original > > > problem... since then I rebased on current master dpdk, got updated to gcc > > > 8.1 and added more patches on lagopus. > > > > > > So just drop this patch if you don't want the bsd lstrcpy. > > > > > Yes, let's drop it from the set for now anyway, if it's not solving any > > specific error. We can relook at it in 18.08 anyway. > > Sorry to immediately contradict myself but I forgot a step in the rather > complicated flow to force the lagopus dpdk submodule HEAD... by default > making lagopus forces the submodule commit to something else. It does need > a much smaller one-liner to avoid > > In file included from ./dpdk/dpdk.c:88: > /projects/lagopus/src/dpdk/build/include/rte_string_fns.h: In > function 'rte_strlcpy': > /projects/lagopus/src/dpdk/build/include/rte_string_fns.h:58:9: > warning: conversion to 'size_t' {aka 'long unsigned int'} from > 'int' may change the sign of the result [-Wsign-conversion] > return snprintf(dst, size, "%s", src); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > It just needs a cast to make the return type of the snprintf compatible with > the expected return type of strlcpy(). > > I'll include this in the next send in an hour or two. > Great, thanks for the all the patches!