From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH] kni: fix build with gcc 8.1 Date: Wed, 20 Jun 2018 18:04:42 +0100 Message-ID: <6bcc0a2c-9dbb-eefb-412c-10fea466e213@intel.com> References: <20180619120855.78207-1-ferruh.yigit@intel.com> <20180619163536.2da8e683@xeon-e3> <90559d85-5fc2-8f77-3b2f-676c9d0b664a@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, stable@dpdk.org To: Stephen Hemminger Return-path: In-Reply-To: <90559d85-5fc2-8f77-3b2f-676c9d0b664a@intel.com> Content-Language: en-US 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 6/20/2018 5:39 PM, Ferruh Yigit wrote: > On 6/20/2018 12:35 AM, Stephen Hemminger wrote: >> On Tue, 19 Jun 2018 13:08:55 +0100 >> Ferruh Yigit wrote: >> >>> Error observed when CONFIG_RTE_KNI_KMOD_ETHTOOL config option is >>> enabled. >>> >>> build error: >>> In function ‘strncpy’, >>> inlined from ‘igb_get_drvinfo’ at >>> .../dpdk/build/build/kernel/linux/kni/igb_ethtool.c:814:2: >>> .../include/linux/string.h:246:9: error: ‘__builtin_strncpy’ output >>> may be truncated copying 31 bytes from a string of length 42 >>> [-Werror=stringop-truncation] >>> return __builtin_strncpy(p, q, size); >>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> Fixed by reducing the adapter->fw_version size and adjusting strncpy >>> limit size. >>> >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Ferruh Yigit >>> --- >>> kernel/linux/kni/ethtool/igb/igb.h | 2 +- >>> kernel/linux/kni/ethtool/igb/igb_ethtool.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/kernel/linux/kni/ethtool/igb/igb.h b/kernel/linux/kni/ethtool/igb/igb.h >>> index 8aa2a3088..5e798ae83 100644 >>> --- a/kernel/linux/kni/ethtool/igb/igb.h >>> +++ b/kernel/linux/kni/ethtool/igb/igb.h >>> @@ -592,7 +592,7 @@ struct igb_adapter { >>> int int_mode; >>> u32 rss_queues; >>> u32 vmdq_pools; >>> - char fw_version[43]; >>> + char fw_version[32]; >> >> Use ETHTOOL_FWVERS_LEN? > > Yes, that is what drvinfo->fw_version uses, and it seems it has been around > enough to not cause any build issue, will use that one. Ah so much fun, it has been updated to 43 by me in the past, to prevent another warning [1] which another part of code has potential to write larger string to fw_version. Reducing value back to 32 may bring back that old warning ... [1] Fixes: c3698192940c ("kni: fix build with gcc 7.1") > >> >>> u32 wvbr; >>> struct igb_mac_addr *mac_table; >>> #ifdef CONFIG_IGB_VMDQ_NETDEV >>> diff --git a/kernel/linux/kni/ethtool/igb/igb_ethtool.c b/kernel/linux/kni/ethtool/igb/igb_ethtool.c >>> index 064528bcf..0b8b25ff1 100644 >>> --- a/kernel/linux/kni/ethtool/igb/igb_ethtool.c >>> +++ b/kernel/linux/kni/ethtool/igb/igb_ethtool.c >>> @@ -812,7 +812,7 @@ static void igb_get_drvinfo(struct net_device *netdev, >>> strncpy(drvinfo->version, igb_driver_version, sizeof(drvinfo->version) - 1); >>> >>> strncpy(drvinfo->fw_version, adapter->fw_version, >>> - sizeof(drvinfo->fw_version) - 1); >>> + sizeof(drvinfo->fw_version)); >> >> Why not: >> strlcpy(drvinfo->fw_version, adapter->fw_version, >> sizeof(drvinfo->fw_version)); > > Just to be cautious about changing API, to not have any problem with older > version of kernels. > But it seems strlcpy supported long enough to not cause an issue, I will use it. >