From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id BBEDA5F4F for ; Tue, 8 May 2018 10:19:14 +0200 (CEST) To: Andrew Rybchenko , dev@dpdk.org References: <152575364588.56689.3300796065057551728.stgit@localhost.localdomain> <152575382842.56689.4589071928538784307.stgit@localhost.localdomain> From: Andy Green Message-ID: Date: Tue, 8 May 2018 16:18:41 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 May 2018 08:19:14 -0000 On 05/08/2018 03:36 PM, Andrew Rybchenko wrote: > Many thanks. I guess the most of below notes are applicable to many other > patches in the series. > > Signed-off-by: ,  Fixes: and Cc: stable@dpdk.org tags are missing. See [1]. Everybody's project has different prejudices. > Changeset summary should start from "net/sfc: " > I.e. something like: > net/sfc: fix strncpy size and NUL Yeah if that's what you like. > (it looks like "another" is useless in the original subject) It captures my feeling at having to wade through making 18 fixes before I could compile the project on current Fedora. > In general all patches should pass ./devtools/check-git-log.sh and > ./devtools/checkpatches.sh > (which requires path to Linux kernel checkpatches.pl). Can you help me understand why adding CRLFs at 80 cols on the gcc errors I pasted helps anything at all? The patches actually fix problems in the code. If you don't care about Coverity, let me know and I will register this project there and send you fixes when I have time. > Andrew. > > [1] > http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line > > On 05/08/2018 07:30 AM, Andy Green wrote: >> --- >> drivers/net/sfc/sfc_ethdev.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c >> index e9bb283e0..bd5f17f33 100644 >> --- a/drivers/net/sfc/sfc_ethdev.c >> +++ b/drivers/net/sfc/sfc_ethdev.c >> @@ -662,10 +662,13 @@ sfc_xstats_get_names(struct rte_eth_dev *dev, >> >> for (i = 0; i < EFX_MAC_NSTATS; ++i) { >> if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) { >> - if (xstats_names != NULL && nstats < xstats_count) >> + if (xstats_names != NULL && nstats < xstats_count) { >> strncpy(xstats_names[nstats].name, >> efx_mac_stat_name(sa->nic, i), >> - sizeof(xstats_names[0].name)); >> + sizeof(xstats_names[0].name) - 1); >> + xstats_names[0].name[ >> + sizeof(xstats_names[0].name) - 1] = '\0'; >> + } > > In fact strlcpy() should be used. Fair enough. Last time I looked it wasn't in glibc but seems it is now. -Andy >> nstats++; >> } >> } >> >