Re: tb/ban-strtok

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 24, 2023, 16:39 UTC
Message-ID: <xmqqsfcpfd5f.fsf@gitster.g>
In-Reply-To: <20230422112213.GE2969939@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

>> I would be curious to get Peff's (cc'd) thoughts on this series, since
>> it was something that he and I were talking about off-list. It was one
>> of those "let me see how hard this would be..." topics, that by the time
>> I finished investigating, I had the series ready to go.
>
> I left a few small comments on the series.
>
> On the greater question of "should strtok or strtok_r be banned", I
> don't have too strong a feeling. The hidden global state in strtok() is
> bad, so probably worth outlawing.

I was and still am inclined to say both can be banned. The primary problem I had with the original series was the way the replacement was sold (namely, string_list based solution is not universally superior but it was sold as if it were, and the sales pitch should have been that in our codebase there is no usecase where strtok_r() is more suitable than string_list_split and friends).

Show 5 quoted lines

> I tend to think that strtok_r() is a bit confusing to use. As Chris
> noted, strsep() is better, but not necessarily as portable. Using
> ptr/len pairs to parse via strcspn(), etc, seems better still. But that
> is mostly aesthetics and preference, so I'm OK if we don't outright ban
> strtok_r().

I am OK if we do ;-)