RE: [PATCH 00/21] replace strtok with strtok_r

["Morten Brørup" <mb@smartsharesystems.com>]

> From: fengchengwen [mailto:fengchengwen@huawei.com]
> Sent: Wednesday, 15 November 2023 04.03
> 
> 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.

+1 to rte_strtok doing the reentrant dance for us.

If we had such an rte_strtok(), we could also generally disallow the use of strtok().

> 
> >
> > what do others think?
> >