From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <andy@warmcat.com> Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id BBEDA5F4F for <dev@dpdk.org>; Tue, 8 May 2018 10:19:14 +0200 (CEST) To: Andrew Rybchenko <arybchenko@solarflare.com>, dev@dpdk.org References: <152575364588.56689.3300796065057551728.stgit@localhost.localdomain> <152575382842.56689.4589071928538784307.stgit@localhost.localdomain> <e2e3bb52-6880-6611-21d8-6be805dfac7b@solarflare.com> From: Andy Green <andy@warmcat.com> Message-ID: <fb7fbdc4-29f4-7fb8-cd72-0fb83af05d28@warmcat.com> 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: <e2e3bb52-6880-6611-21d8-6be805dfac7b@solarflare.com> 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 <dev.dpdk.org> List-Unsubscribe: <https://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=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++; >> } >> } >> >