From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: [PATCH v5 02/21] rte_string_fns.h: fix gcc8.1 sign conv warning in lstrcpy Date: Thu, 17 May 2018 16:28:27 +0100 Message-ID: <20180517152827.GA23992@bricha3-MOBL.ger.corp.intel.com> References: <152656480225.46638.3271983577765861155.stgit@localhost.localdomain> <152656494207.46638.7698825480823239153.stgit@localhost.localdomain> <20180517074016.7873db83@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andy Green , dev@dpdk.org To: Stephen Hemminger Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id D6E0E7F14 for ; Thu, 17 May 2018 17:28:43 +0200 (CEST) Content-Disposition: inline In-Reply-To: <20180517074016.7873db83@xeon-e3> 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 07:40:16AM -0700, Stephen Hemminger wrote: > On Thu, 17 May 2018 21:49:02 +0800 > Andy Green wrote: > > > 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); > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > --- > > lib/librte_eal/common/include/rte_string_fns.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h > > index fcbb42e00..97597a148 100644 > > --- a/lib/librte_eal/common/include/rte_string_fns.h > > +++ b/lib/librte_eal/common/include/rte_string_fns.h > > @@ -55,7 +55,7 @@ rte_strsplit(char *string, int stringlen, > > static inline size_t > > rte_strlcpy(char *dst, const char *src, size_t size) > > { > > - return snprintf(dst, size, "%s", src); > > + return (size_t)snprintf(dst, size, "%s", src); > > } > > > > /* pull in a strlcpy function */ > > > > I still like the BSD function better because it guarantees all data > in the buffer is zero'd snprintf does not. Right. But that is really a separate change from just fixing compiler errors, which is why I think it's best kept out of this set. As for zero-ing the rest of the buffer, reading the man page for strlcpy on my fedora system, I find no mention of that behaviour. This implies to me that the only guarantee from strlcpy is that there is one zero byte at the end, and that the zeroing the rest of the array is not to be relied up. Therefore, I see little value in having it in the fallback implementation unless we are going to guarantee that we always use that implementation - something we can't really do on freebsd systems, for example. /Bruce