Junio C Hamano wrote:
Wang Hui <Hui.Wang@xxxxxxxxxxxxx> writes:
From: Hui Wang <Hui.Wang@xxxxxxxxxxxxx>
The ent->base[] is a character array, it has pfxlen characters from
position 0 to (pfxlen-1) to contain an alt object dir name, the
position pfxlen should be the string terminating character '\0' and
is deliberately set to '\0' at the previous code line. The position
(pfxlen+1) is given to ent->name.
Correct. Do you understand why?
We temporarily NUL terminate the ent->base[] so that we can give it to
is_directory() to see if that is a directory, but the invariants for a
alternate_object_database instance after it is properly initialized by
this function are to have:
- the directory name followed by a slash in the base[] array;
- the name pointer pointing at one byte beyond the slash;
- name[2] filled with a slash; and
- name[41] terminated with NUL.
Later, has_loose_object_nonlocal() calls fill_sha1_path() with the name
pointer to fill name[0..1, 3..40] with the hexadecimal representation of
the object name, which would result in base[] array to have the pathname
for a loose object found in that alternate. The same thing happens in
open_sha1_file() to read from a loose object in an alternate.
And you are breaking one of the above invariants by removing that slash
after the directory name. These callers of fill_sha1_path() will see the
directory name, your NUL, two hex, slash, and 38 hex in base[].
Understand now, thanks for your explanation.
How would the code even work with your patch?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html