From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH 4/5] replace snprintf with strlcpy without adding extra include Date: Thu, 04 Apr 2019 20:05:06 +0200 Message-ID: <4698261.tayNczNUhG@xps> References: <20190403144505.46234-1-bruce.richardson@intel.com> <20190403085132.1fa33194@shemminger-XPS-13-9360> <20190403155638.GA1336@bricha3-MOBL.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, Stephen Hemminger To: Bruce Richardson Return-path: Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 87DCB1B44E for ; Thu, 4 Apr 2019 20:05:09 +0200 (CEST) In-Reply-To: <20190403155638.GA1336@bricha3-MOBL.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 03/04/2019 17:56, Bruce Richardson: > On Wed, Apr 03, 2019 at 08:51:32AM -0700, Stephen Hemminger wrote: > > On Wed, 3 Apr 2019 15:45:04 +0100 > > Bruce Richardson wrote: > > > > > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c > > > index d215acecc..a542f6f5d 100644 > > > --- a/lib/librte_ring/rte_ring.c > > > +++ b/lib/librte_ring/rte_ring.c > > > @@ -78,7 +78,7 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count, > > > > > > /* init the ring structure */ > > > memset(r, 0, sizeof(*r)); > > > - ret = snprintf(r->name, sizeof(r->name), "%s", name); > > > + ret = strlcpy(r->name, name, sizeof(r->name)); > > > if (ret < 0 || ret >= (int)sizeof(r->name)) > > > > I would rather use the name length that is part of the header file. > > > > if (strnlen(name, RTE_RING_NAMESIZE) == RTE_RING_NAMESIZE) > > return -ENAMETOOLONG; > > > > strlcpy(r->name, name, sizeof(r->name)) > > > Yes, though honestly it's a matter of preference. Since this was a scripted > replacement each change wasn't checked in too much detail other than a > cursory check for correctness. I prefer using sizeof/strlen rather than the constant used for allocation. Reasons are: - it's easier to understand the intent - it's easier to change how the allocation size is chosen