Re: [PATCH] Use FIX_UTF8_MAC to enable conversion from UTF8-MAC to UTF8

On Mon, 21 Jan 2008, Junio C Hamano wrote:

Individual components are limited to 255 bytes by most filesystems (PATH_MAX is the whole path, not any individual component).

That said, you're right. It's not really a design requirement, and since you never get an array of "struct dirent", just a pointer to a single one, it would be perfectly normal and natural for "struct dirent" to be declared with a unsized d_name[].

It's also quite possible that some implementations might even have d_name[] not as an array, but as a pointer to somewhere else (POSIX may require it to be an array, I didn't check).

That said, I bet that Mark isn't the only one to have written code like that, so I suspect Mark's code probably works in practice pretty much everywhere, even if I don't think it's necessarily _required_ to work correctly.

I do suspect that if you really want to make this portable, and able to handle an expanding d_name[] too, I think you need to make sure you allocate a big-enough one. And if you worry about d_name perhaps being a pointer, that really does mean that you'd need to convert the system-supplied "struct dirent" into a "git_dirent_t" that you can control.

That said, I think this patch has a bigger problem, namely just fundamentally that

	char *utf8 = reencode_string(entry, "UTF8", "UTF8-MAC");

is just unbelievably slow. That's just not how it should be done.

First off, the common case is that the filename likely has everything in plain 7-bit ascii. So rather than re-encoding by default, the first thing to do is to just see if it even needs re-encoding. Even if it's as simple as saying "does it have any high bits at all", that's going to be a *huge* performance win.

So start off with something like

	int is_usascii(const char *p)
	{
		char c;

		do {
			c = *p++;
		} while (c > 0);
		return !c;
	}

and now you can do

	if (is_usascii(entry->d_name))
		return entry;

before you even *look* at re-encoding it (and this basically works for all cases - we really don't care about EBCDIC, do we? So even if this routine was meant to do Latin1<->utf8, the above "is_usascii()" test is always the right thing to do).

Anyway, even if you do that, our "reencode_string()" is really *so* expensive that you really don't want to do it on a filename by filename basis. It literally does a malloc() for each allocation. It might well be worth it to find something that is more utf-8-specific (and I could well imagine that Mac OS X comes with some UTF libraries, if only because we cannot possibly be the only people with this issue).

(Same goes for Latin1<->UTF conversion, for that matter. If somebody wants to add that, I suspect it's best done by hand, not using iconv and our rather expensive layer around it. That said, latin1->utf8 is actually much *easier* than utf-8 NFD->NFC).

			Linus