Re: Use of strtok() in dpdk code

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 22 Oct 2024 14:51:39 +0800
fengchengwen <fengchengwen@huawei.com> wrote:

> On 2024/10/22 9:08, Stephen Hemminger wrote:
> > On Mon, 21 Oct 2024 21:30:02 +0300
> > Isaac Boukris <iboukris@gmail.com> wrote:
> >   
> >> Hello,
> >>
> >> I was debugging a crash resulting from strtok() returning NULL
> >> unexpectedly (string still had tokens and delimiters), and the only
> >> explanation I could come up with was that strtok is thread-unsafe and
> >> another thread could have been calling it at the same time, and so I
> >> changed it to use strtok_r().
> >>
> >> That said, the only other possible use of strtok() that I could find
> >> was in the dpdk code (telemetry), which brings me to my question,
> >> should we consider changing all occurrences to strtok_r() or am I
> >> missing something? there seem to be quite some in non-initialization
> >> code.
> >>
> >> Thanks!  
> > 
> > 
> > Most of the uses are in tests and other single threaded code.
> > In general, simpler just to use strtok_r everywhere and not worry about it.
> > Similar to not using sprintf() and instead using snprintf().  
> 
> I'm afraid I can't agree.
> 
> DPDK is just a SDK, it's not an application (although DPDK provided simple examples).
> Many code will developped based on DPDK, we can't predict how it was implemented.
> So there maybe a DPDK thread and a application thread both invoke strtok().
> 
> From this point of view, I hope that DPDK solves some of the reentrant problems of
> such C functions (e.g. strtok()\strerror()).
> 
> Actually, we've try to solve before, but unfortunately it wasn't merged.
> 1\ strtok():   https://inbox.dpdk.org/dev/20231114110006.91148-1-haijie1@huawei.com/T/#u
> 2\ strerror(): https://inbox.dpdk.org/dev/20231114123552.398072-1-huangdengdui@huawei.com/T/#u
> 
> > 
> > Some code scanners like codeql also flag this.  
> 

The usages of strtok() and strerror() in init code are fine. Stuff only called from
eal_init() like devargs should be safe, but fixing it makes sense.

Perhaps a coccinelle script could help here.