From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 1938B4332D;
	Tue, 14 Nov 2023 18:34:36 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id DD60F4028A;
	Tue, 14 Nov 2023 18:34:35 +0100 (CET)
Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182])
 by mails.dpdk.org (Postfix) with ESMTP id 9502F4027B
 for <dev@dpdk.org>; Tue, 14 Nov 2023 18:34:34 +0100 (CET)
Received: by linux.microsoft.com (Postfix, from userid 1086)
 id AA7BE20B74C1; Tue, 14 Nov 2023 09:34:33 -0800 (PST)
DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AA7BE20B74C1
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com;
 s=default; t=1699983273;
 bh=3e++kA3nEn/WWIiTEjLici3ne1RQBiHawLe4JVSx0zU=;
 h=Date:From:To:Cc:Subject:References:In-Reply-To:From;
 b=f3Zwhc+wRhJ8obtX4/RStJglx+y1ulo6MUi1k27DtbnM+054WJhxNHWqct103YUkV
 u109Qbhd4ZGkEefqsw7UIVya7nYydFI/VTNqNwG/AMsEQxNwtakWfYPFOvWwU/0hFH
 3Wzz3FxloIed3sHRjSoda8Mz8vobQ0/K2LCU3sCM=
Date: Tue, 14 Nov 2023 09:34:33 -0800
From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: Jie Hai <haijie1@huawei.com>
Cc: dev@dpdk.org, lihuisong@huawei.com, fengchengwen@huawei.com
Subject: Re: [PATCH 00/21] replace strtok with strtok_r
Message-ID: <20231114173433.GB23774@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <20231113104550.2138654-1-haijie1@huawei.com>
 <20231113170928.GD13153@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
 <bc7a4c9e-7ec8-0337-f36c-1e95294059ed@huawei.com>
 <20231114173248.GA23774@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231114173248.GA23774@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Tue, Nov 14, 2023 at 09:32:48AM -0800, Tyler Retzlaff wrote:
> On Tue, Nov 14, 2023 at 08:50:17PM +0800, Jie Hai wrote:
> > On 2023/11/14 1:09, Tyler Retzlaff wrote:
> > >On Mon, Nov 13, 2023 at 06:45:29PM +0800, Jie Hai wrote:
> > >>Multiple threads calling the same function may cause condition
> > >>race issues, which often leads to abnormal behavior and can cause
> > >>more serious vulnerabilities such as abnormal termination, denial
> > >>of service, and compromised data integrity.
> > >>
> > >>The strtok() is non-reentrant, it is better to replace it with a
> > >>reentrant function.
> > >
> > >could you please use strtok_s instead of strtok_r the former is part of
> > >the C11 standard the latter is not.
> > >
> > >thanks!
> > >
> > Hi, Tyler Retzlaff
> > 
> > Thanks for your comment.
> > 
> > For the use of strtok_s, I have consulted some documents, see
> > 	https://en.cppreference.com/w/c/string/byte/strtok
> > It says
> > "As with all bounds-checked functions, strtok_s only guaranteed to be
> > available if __STDC_LIB_EXT1__ is defined by the implementation and if
> > the user defines __STDC_WANT_LIB_EXT1__ to the integer constant 1 before
> > including <string.h>.
> > "
> > 
> > I use strtok_s with "#ifdef __STDC_LIB_EXT1__ ... #endif" around and
> > compiled
> > locally and found that __STDC_LIB_EXT1__ was not defined, so the related
> > code  was not called. I'm afraid there's a problem with this
> > modification.
> > 
> > Am I using strtok_s wrong?
> 
> no i overlooked that it is optional my fault sorry.
> 
> since there is no portable strtok_r i'm going to have to agree with others
> that only places where you actually need reentrant strtok be converted to
> strtok_r.
> 
> for windows i think we'll either need to introduce an abstracted strtok_r
> name for portability or something in the rte_ namespace (dependent on
> what other revieweres would prefer)

just as a follow up here maybe it would be optimal if we could use
strtok_s *if* we test that the toolchain makes it available and if not
provide a mapping of strtok_s -> strtok_r.

what do others think?

> 
> thanks!
> 
> > 
> > Thanks,
> > Jie Hai
> > >>
> > >>Jie Hai (21):
> > >>   app/graph: replace strtok with strtok_r
> > >>   app/test-bbdev: replace strtok with strtok_r
> > >>   app/test-compress-perf: replace strtok with strtok_r
> > >>   app/test-crypto-perf: replace strtok with strtok_r
> > >>   app/test-dma-perf: replace strtok with strtok_r
> > >>   app/test-fib: replace strtok with strtok_r
> > >>   app/dpdk-test-flow-perf: replace strtok with strtok_r
> > >>   app/test-mldev: replace strtok with strtok_r
> > >>   lib/dmadev: replace strtok with strtok_r
> > >>   lib/eal: replace strtok with strtok_r
> > >>   lib/ethdev: replace strtok with strtok_r
> > >>   lib/eventdev: replace strtok with strtok_r
> > >>   lib/telemetry: replace strtok with strtok_r
> > >>   lib/telemetry: replace strtok with strtok_r
> > >>   bus/fslmc: replace strtok with strtok_r
> > >>   common/cnxk: replace strtok with strtok_r
> > >>   event/cnxk: replace strtok with strtok_r
> > >>   net/ark: replace strtok with strtok_r
> > >>   raw/cnxk_gpio: replace strtok with strtok_r
> > >>   examples/l2fwd-crypto: replace strtok with strtok_r
> > >>   examples/vhost: replace strtok with strtok_r
> > >>
> > >>  app/graph/graph.c                             |  5 ++-
> > >>  app/graph/utils.c                             | 15 +++++---
> > >>  app/test-bbdev/test_bbdev_vector.c            | 25 +++++++-----
> > >>  .../comp_perf_options_parse.c                 | 16 ++++----
> > >>  app/test-crypto-perf/cperf_options_parsing.c  | 16 ++++----
> > >>  .../cperf_test_vector_parsing.c               | 10 +++--
> > >>  app/test-dma-perf/main.c                      | 13 ++++---
> > >>  app/test-fib/main.c                           | 10 ++---
> > >>  app/test-flow-perf/main.c                     | 22 ++++++-----
> > >>  app/test-mldev/ml_options.c                   | 18 ++++-----
> > >>  drivers/bus/fslmc/fslmc_bus.c                 |  5 ++-
> > >>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c      |  4 +-
> > >>  drivers/common/cnxk/cnxk_telemetry_nix.c      | 12 +++---
> > >>  drivers/event/cnxk/cnxk_eventdev.c            | 10 +++--
> > >>  drivers/event/cnxk/cnxk_tim_evdev.c           | 11 +++---
> > >>  drivers/net/ark/ark_pktchkr.c                 | 10 ++---
> > >>  drivers/net/ark/ark_pktgen.c                  | 10 ++---
> > >>  drivers/raw/cnxk_gpio/cnxk_gpio.c             |  6 +--
> > >>  examples/l2fwd-crypto/main.c                  |  6 +--
> > >>  examples/vhost/main.c                         |  3 +-
> > >>  lib/dmadev/rte_dmadev.c                       |  4 +-
> > >>  lib/eal/common/eal_common_memory.c            |  8 ++--
> > >>  lib/ethdev/rte_ethdev_telemetry.c             |  6 ++-
> > >>  lib/eventdev/rte_event_eth_rx_adapter.c       | 38 +++++++++----------
> > >>  lib/eventdev/rte_eventdev.c                   | 18 ++++-----
> > >>  lib/security/rte_security.c                   |  3 +-
> > >>  lib/telemetry/telemetry.c                     |  5 ++-
> > >>  27 files changed, 169 insertions(+), 140 deletions(-)
> > >>
> > >>-- 
> > >>2.30.0
> > >.