Subject: Re: [PATCH 5/5] banned.h: mark `strtok()`, `strtok_r()` as banned

From: Junio C Hamano <[email-masked]>
Date: Thu, 13 Apr 2023 18:39:18 -0700

Taylor Blau <[email-masked]> writes:

>   - `strtok_r()` forces the caller to maintain an extra string pointer
>     to pass as its `saveptr` value
>
>   - `strtok_r()` also requires that its `saveptr` value be unmodified
>     between calls.
>
>   - `strtok()` (and by extension, `strtok_r()`) is confusing when used
>     across multiple functions, since the caller is supposed to pass NULL
>     as its first argument after the first call. This makes it difficult
>     to determine what string is actually being tokenized without clear
>     dataflow.

It seems that the only existing users of strtok() are all in
t/helper/ directory, so I personally do not think it is a huge loss
if these two are forbidden.  While I do not see why we should use
strtok(), none of the above sound like sensible reasons to ban
strtok_r().  At best, they may point out awkwardness of the function
to make you try finding an alternative that is easier-to-use before
choosing strtok_r() for your application on a case-by-case basis.

If your application wants to chomp a string into tokens from left to
right, inspecting the resulting token one-by-one as it goes until it
hits a token that satisfies some condition and then terminate
without wasting cycles on the rest, string_list_split_in_place() is
a poor choice.  In such a use case, you do not know upfront where in
the string the sought-after token would be, so you have to split the
string in full without taking an early exit via maxsplit.  Also, you
are restricted to a single byte value for the delimiter, and unlike
strtok[_r](), string_list_split_in_place() does not squash a run of
delimiter bytes into one inter-token delimiter.

One gripe I have against use of strtok() is actually not with
threading but because people often misuse it when strcspn() is what
they want (i.e. measure the length of the "first token", so that
they can then xmemdupz() a copy out), forgetting that strtok[_r]()
is destructive.

So, I dunno.