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

Mark Junker [off-list ref] writes:

Junio C Hamano schrieb:

quoted

I do not know how Macintosh libc implements "struc dirent", but
this approach does not work in general.

IMHO there is no need that this approach works in general because this
is a fix for MacOSX systems only. I also use d_namlen which might not
be available on other systems. But on MacOSX this works as expected.

quoted

yet you can obtain a path component longer than 256 bytes.
Apparently the library allocates longer d_name[] field than what
is shown to the user.

This is not a problem either because on MacOSX we get decomposed UTF8
and we always convert to composed UTF8. This means that the string
returned from reencode_string will always be smaller than the original
filename that had to be reencoded.

It is not quite enough that this works Ok on MacOS, if you made
FIX_UTF8_MAC definable in the Makefile.  After all some friendly
and helpful Linux folks might want to enable it with their build
trying to help debugging, right?

In the short term, as long as it safely runs without overrunning
the buffer on MacOS, then that is fine, even though we will need
some protection to prevent this code from getting compiled and
used on Linux with glibc, which does have the issue.

I was specifically talking about this "static" thing.

+static struct dirent temp;


+struct dirent *gitreaddir(DIR *dirp)
+{
+	size_t utf8_len;
+	char *utf8;
+	struct dirent *result;
+	result = readdir(dirp);
+	if (result != NULL) {
+		memcpy(&temp, result, sizeof(struct dirent));
+		utf8 = reencode_string(temp.d_name, "UTF8", "UTF8-MAC");
+		if (utf8 != NULL) {
+			utf8_len = strlen(utf8);
+			temp.d_namlen = (u_int8_t) utf8_len;
+			memcpy(temp.d_name, utf8, utf8_len + 1);
+			free(utf8);
+			result = &temp;
+		}
+	}
+	return result;
+}

You memcpy() what the library gave you in *result to the
statically allocated "temp".  d_name[] in "temp" comes from the
structure definition in the user visible include file, which
could be much shorter than what the library gave you in *result.
The structure definition I showed in my message you are
responding to illustrates the issue.  If MacOS uses a similar
trick to define d_name[256] and sometimes returns much longer
name in *result, you are truncating the name by copying only the
first part of the structure and first 256 bytes of d_name[]. 

But you have a Mac, I don't, so as long as you have verified
that their header has enough room in statically allocated "temp"
to store longest possible name that can be returned from
readdir(), the code is Ok.  I was just being cautious, as I know
the above code has a problem on one platform.

Junio C Hamano schrieb:

You memcpy() what the library gave you in *result to the
statically allocated "temp".  d_name[] in "temp" comes from the
structure definition in the user visible include file, which
could be much shorter than what the library gave you in *result.
The structure definition I showed in my message you are
responding to illustrates the issue.  If MacOS uses a similar
trick to define d_name[256] and sometimes returns much longer
name in *result, you are truncating the name by copying only the
first part of the structure and first 256 bytes of d_name[]. 

Now I understand what you mean. Ok, I'll try to change this and make 
this work on other platforms too.

I didn't know that the readdir function is allowed to return something 
longer for d_name than the specified length.

Regards,
Mark