Re: [PATCH] refs/packed: fix BUG when seeking refs with UTF-8 characters

From: Jeff King <peff@peff.net>
Date: Apr 8, 2025, 06:46 UTC
Message-ID: <20250408064619.GA1957411@coredump.intra.peff.net>
In-Reply-To: <Z_CJnBWReJeZKC9x@tapette.crustytoothpaste.net>

On Sat, Apr 05, 2025 at 01:38:36AM +0000, brian m. carlson wrote:

Show 10 quoted lines

> > Ah, good catch. I think this signed-ness issue has come up before, long
> > ago, but I don't remember the context. In theory any stable ordering is
> > OK for sorting, but of course cmp_packed_refname() chose to use unsigned
> > in order to match strcmp(), and the standard defines it as interpreting
> > the bytes as unsigned. One of the enjoyable quirks of C.
> 
> I'd agree it's a quirk, but I'm not sure I'd call it enjoyable.  Anyway,
> this does seem like the right solution and I agree matching strcmp is
> the right decision here.  I also think unsigned byte comparisons are
> more intuitive, honestly.

My sarcasm may have gotten lost in the wires. ;)

And yes, I think unsigned byte comparisons are more intuitive, too. The fact that "char" is (or according to the standard _might_ be) signed is confusing, especially in a world of unicode.

Show 14 quoted lines

> > It's possible some filesystems might be unhappy with this character, but
> > I guess we can see if anybody screams.
> 
> I very much doubt that's going to be a problem.  Windows uses UTF-16
> internally, so it will have no problems with Unicode; macOS only allows
> UTF-8 in its file systems and I know it gracefully supports emoji; and
> other Unix systems don't care one way or the other because it's just
> some bytes.  Even ISO9660 with Rock Ridge or Joliet extensions will work
> here, as will FAT, UDF (used on DVDs and some hard disks), and 9P.
> 
> Certainly somebody could try something very esoteric, but I expect other
> things will break as well.  I'm okay with favouring testing things we
> know many people _are_ using (emojis and Unicode) over things very few
> people are using (very esoteric file systems[0]).

Good point that people are doing this in practice anyway. The worst case is probably that we flush out some interesting limitations, learn more about the world, and add a small prereq flag to the test.

-Peff