Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Taylor Blau <me@ttaylorr.com>, git@vger.kernel.org
Subject: Re: tb/ban-strtok
Date: Mon, 24 Apr 2023 09:39:08 -0700	[thread overview]
Message-ID: <xmqqsfcpfd5f.fsf@gitster.g> (raw)
In-Reply-To: <20230422112213.GE2969939@coredump.intra.peff.net> (Jeff King's message of "Sat, 22 Apr 2023 07:22:13 -0400")

Jeff King <peff@peff.net> writes:

>> 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).

> 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 ;-)

  reply	other threads:[~2023-04-24 16:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-20 22:34 What's cooking in git.git (Apr 2023, #06; Thu, 20) Junio C Hamano
2023-04-21  2:16 ` tb/ban-strtok (was: Re: What's cooking in git.git (Apr 2023, #06; Thu, 20)) Taylor Blau
2023-04-22 11:22   ` Jeff King
2023-04-24 16:39     ` Junio C Hamano [this message]
2023-04-22 15:00 ` en/header-split-cache-h-part-2 (Was: " Elijah Newren
2023-04-24 18:20   ` en/header-split-cache-h-part-2 Junio C Hamano
2023-04-22 15:00 ` pb/complete-and-document-auto-merge-and-friends (Was: Re: What's cooking in git.git (Apr 2023, #06; Thu, 20)) Elijah Newren
2023-04-22 21:53   ` Philippe Blain
2023-04-24 18:21   ` pb/complete-and-document-auto-merge-and-friends Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2023-04-18  8:12 What's cooking in git.git (Apr 2023, #05; Mon, 17) Junio C Hamano
2023-04-18 14:56 ` tb/ban-strtok (was: Re: What's cooking in git.git (Apr 2023, #05; Mon, 17)) Taylor Blau
2023-04-18 16:58   ` tb/ban-strtok Junio C Hamano
2023-04-18 19:31     ` tb/ban-strtok Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsfcpfd5f.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).