From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id B3D0F1B450 for ; Wed, 3 Apr 2019 16:57:52 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Apr 2019 07:57:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,304,1549958400"; d="scan'208";a="132656786" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga006.jf.intel.com with ESMTP; 03 Apr 2019 07:57:51 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 3 Apr 2019 07:57:51 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.142]) by FMSMSX154.amr.corp.intel.com ([169.254.6.237]) with mapi id 14.03.0415.000; Wed, 3 Apr 2019 07:57:50 -0700 From: "Wiles, Keith" To: "Richardson, Bruce" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 0/5] clean up snprintf use for string copying Thread-Index: AQHU6ivsTgsdTBAINEa+aarG3SRS86Yq/DUA Date: Wed, 3 Apr 2019 14:57:49 +0000 Message-ID: <5F7D9EB2-AD8D-475B-852C-D8B25475F25C@intel.com> References: <20190403144505.46234-1-bruce.richardson@intel.com> In-Reply-To: <20190403144505.46234-1-bruce.richardson@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.102.28] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 0/5] clean up snprintf use for string copying 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: Wed, 03 Apr 2019 14:57:53 -0000 > On Apr 3, 2019, at 9:45 AM, Bruce Richardson = wrote: >=20 > There are quite a few instances remaining in DPDK where snprintf is being > used for string copying. These were not being picked up by our existing > coccinelle script, but that can be fixed by editing the script and runnin= g > it against our code. In the process a bug was found and fixed in the > bonding pmd, where we were incorrectly specifiying the buffer length > parameter to snprintf. >=20 > The actual replacement was done in two phases - first replacing all > instances where only the snprintf line in question needed changing, then > fixing the other instances where we also needed to add in the header > include. [Using two stages allowed the header addition to be automated to= o, > since we had a list of files where every one needed the header inclusion] >=20 >=20 > Bruce Richardson (5): > net/bonding: fix buffer length when printing strings > devtools/cocci: make strlcpy replacement smarter > devtools/cocci: create safer version of strlcpy script > replace snprintf with strlcpy without adding extra include > replace snprintf with strlcpy >=20 Should we not be testing the return values from strlcpy and snprintf, which= means we need to create a macro or inline function. We could use a macro a= nd only enable with DEBUG support if we think performance or code size if a= problem. I am surprised none of the tools are catching these types of problems. Not to make Bruce do that change for this patch, but we need to look at it = for a later patch IMO. Regards, Keith From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id EE527A0679 for ; Wed, 3 Apr 2019 16:57:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9BEE21B475; Wed, 3 Apr 2019 16:57:54 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id B3D0F1B450 for ; Wed, 3 Apr 2019 16:57:52 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Apr 2019 07:57:51 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,304,1549958400"; d="scan'208";a="132656786" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga006.jf.intel.com with ESMTP; 03 Apr 2019 07:57:51 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 3 Apr 2019 07:57:51 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.142]) by FMSMSX154.amr.corp.intel.com ([169.254.6.237]) with mapi id 14.03.0415.000; Wed, 3 Apr 2019 07:57:50 -0700 From: "Wiles, Keith" To: "Richardson, Bruce" CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 0/5] clean up snprintf use for string copying Thread-Index: AQHU6ivsTgsdTBAINEa+aarG3SRS86Yq/DUA Date: Wed, 3 Apr 2019 14:57:49 +0000 Message-ID: <5F7D9EB2-AD8D-475B-852C-D8B25475F25C@intel.com> References: <20190403144505.46234-1-bruce.richardson@intel.com> In-Reply-To: <20190403144505.46234-1-bruce.richardson@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.102.28] Content-Type: text/plain; charset="UTF-8" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 0/5] clean up snprintf use for string copying 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190403145749.llS2TmOTphPm_--_htskx03FU1acj3ceUxe9D6-06Ow@z> > On Apr 3, 2019, at 9:45 AM, Bruce Richardson = wrote: >=20 > There are quite a few instances remaining in DPDK where snprintf is being > used for string copying. These were not being picked up by our existing > coccinelle script, but that can be fixed by editing the script and runnin= g > it against our code. In the process a bug was found and fixed in the > bonding pmd, where we were incorrectly specifiying the buffer length > parameter to snprintf. >=20 > The actual replacement was done in two phases - first replacing all > instances where only the snprintf line in question needed changing, then > fixing the other instances where we also needed to add in the header > include. [Using two stages allowed the header addition to be automated to= o, > since we had a list of files where every one needed the header inclusion] >=20 >=20 > Bruce Richardson (5): > net/bonding: fix buffer length when printing strings > devtools/cocci: make strlcpy replacement smarter > devtools/cocci: create safer version of strlcpy script > replace snprintf with strlcpy without adding extra include > replace snprintf with strlcpy >=20 Should we not be testing the return values from strlcpy and snprintf, which= means we need to create a macro or inline function. We could use a macro a= nd only enable with DEBUG support if we think performance or code size if a= problem. I am surprised none of the tools are catching these types of problems. Not to make Bruce do that change for this patch, but we need to look at it = for a later patch IMO. Regards, Keith