From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id A53ED6CD0 for ; Tue, 8 May 2018 11:08:24 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id F29EE60005D; Tue, 8 May 2018 09:08:22 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Tue, 8 May 2018 10:08:18 +0100 To: Andy Green , References: <152575364588.56689.3300796065057551728.stgit@localhost.localdomain> <152575382842.56689.4589071928538784307.stgit@localhost.localdomain> From: Andrew Rybchenko Message-ID: Date: Tue, 8 May 2018 12:08:14 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23830.003 X-TM-AS-Result: No--14.913600-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1525770503-v1LDEdsEokOV Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 09:08:25 -0000 On 05/08/2018 11:18 AM, Andy Green wrote: > On 05/08/2018 03:36 PM, Andrew Rybchenko wrote: >> (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. I see. >> 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. Seeing GCC errors which patch fixes is useful to see. Yes, I agree that it is real problem 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. dpdk is registered at Coverity and we get reports from time to 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. As far as I know it is not in glibc, but dpdk has internal fallback if the function is not available from external libs. Andrew.