Re: [PATCH v4 00/13] replace strtok with reentrant version

[David Marchand <david.marchand@redhat.com>]

Hello,

On Sat, Oct 26, 2024 at 12:26 PM Jie Hai <haijie1@huawei.com> 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.
>
> This patchset replaces strtok with strtok_r in app, example, lib
> and drivers. And adds check for use of strtok in checkpatches.sh.

- The current v4 series breaks compilation on Windows with clang.
http://mails.dpdk.org/archives/test-report/2024-November/819978.html

The reason is that some include of rte_os_shim.h are missing.

==== 20 line log output for Windows Server 2022 (dpdk_win_llvm_compile): ====
vchan_param = strtok_r(end_param, ",", &sp);
^
../lib/dmadev/rte_dmadev.c:1038:17: note: did you mean 'strtok_s'?
C:\Program Files (x86)\Windows
Kits\10\Include\10.0.22621.0\ucrt\string.h:68:29: note: 'strtok_s'
declared here
_ACRTIMP char*  __cdecl strtok_s(
^
../lib/dmadev/rte_dmadev.c:1038:15: error: incompatible integer to
pointer conversion assigning to 'const char *' from 'int'
[-Wint-conversion]
vchan_param = strtok_r(end_param, ",", &sp);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.
[10/836] Linking static target lib/librte_gpudev.a
[11/836] Compiling C object
lib/librte_mldev.a.p/mldev_mldev_utils_scalar_bfloat16.c.obj
[12/836] Compiling C object lib/librte_gro.a.p/gro_gro_udp4.c.obj
[13/836] Compiling C object lib/librte_mldev.a.p/mldev_mldev_utils_scalar.c.obj
[14/836] Compiling C object lib/librte_gro.a.p/gro_rte_gro.c.obj
[15/836] Compiling C object lib/librte_gro.a.p/gro_gro_tcp6.c.obj
[16/836] Compiling C object lib/librte_gro.a.p/gro_gro_tcp4.c.obj
[17/836] Compiling C object lib/librte_gro.a.p/gro_gro_vxlan_tcp4.c.obj
[18/836] Compiling C object
lib/librte_cryptodev.a.p/cryptodev_rte_cryptodev.c.obj
ninja: build stopped: subcommand failed.


- grep shows that there are a number of missed places in drivers and
app, which contradicts the commitlog.
I don't like leaving some places with strtok while checkpatches.sh
warn on the rest.
And I think others were expecting too that the whole tree is fixed
after this series.

Re-reading the thread, were the changes on app/ dropped, following
comments from Stephen?
I understand his request was to remove the backport request (iow no
Cc: stable@dpdk.org in the commitlogs), not to drop the changes.


-- 
David Marchand