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 6A8F143332; Wed, 15 Nov 2023 04:02:50 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 11231402C3; Wed, 15 Nov 2023 04:02:50 +0100 (CET) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id ACFD44027B for <dev@dpdk.org>; Wed, 15 Nov 2023 04:02:48 +0100 (CET) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.56]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4SVSXM0Lr7zMmxr; Wed, 15 Nov 2023 10:58:11 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Wed, 15 Nov 2023 11:02:46 +0800 Subject: Re: [PATCH 00/21] replace strtok with strtok_r To: Tyler Retzlaff <roretzla@linux.microsoft.com>, Jie Hai <haijie1@huawei.com> CC: <dev@dpdk.org>, <lihuisong@huawei.com> 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> <20231114173433.GB23774@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20231114174916.GD23774@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: fengchengwen <fengchengwen@huawei.com> Message-ID: <1e636881-c543-b23b-4f75-499b175db058@huawei.com> Date: Wed, 15 Nov 2023 11:02:46 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <20231114174916.GD23774@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected 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 2023/11/15 1:49, Tyler Retzlaff wrote: > On Tue, Nov 14, 2023 at 09:34:33AM -0800, Tyler Retzlaff wrote: >> 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. > > just a final follow up, i can see that we already have a rte_strerror > here to do the replace with reentrant dance. it is probably good to > follow the already established pattern for this and have a rte_strtok. +1 for have rte_strtok which could cover different platform. > > 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 >>>>> . > . >