On Thu, May 29, 2014 at 5:11 AM, Pasha Bolokhov
<pasha.bolokhov@xxxxxxxxx> wrote:
>>> + len = strlen(n_git); /* real_path() has stripped trailing slash */
>>> + for (i = len - 1; i > 0 && !is_dir_sep(n_git[i]); i--) ;
>>> + basename = n_git + i;
>>> + if (is_dir_sep(*basename))
>>> + basename++;
>>> + if (strcmp(basename, ".git")) {
>>
>> I think normalize_path_copy makes sure that dir sep is '/', so this
>> code may be simplified to "if (strcmp(n_git, .git") && (len == 4 ||
>> strcmp(n_git + len - 5, "/.git")))"?
>
> Then if "n_git" is "/ab" => coredump. But I agree there is logic to
> this (if we check len >= 4 first). However, we still need the
> basename.
Ah I missed this at add_exclude()
> So I've just shortened it a bit, what do you think: (notice
> the condition "i >= 0" btw)
>
> for (i = len - 1; i >= 0 && n_git[i] != '/'; i--) ;
There's basename() that does this for you. A compat version is
provided for Windows port so no portability worries.
> basename = n_git + i + 1;
> if (strcmp(basename, ".git)) {
>
>>
>>> + const char *worktree = get_git_work_tree();
>>> +
>>> + if (worktree == NULL ||
>>> + dir_inside_of(n_git, get_git_work_tree()) >= 0) {
>>> + struct exclude_list *el = add_exclude_list(dir, EXC_CMDL,
>>> + "GIT_DIR setup");
>>> +
>>> + /* append a trailing slash to exclude directories */
>>> + n_git[len] = '/';
>>> + n_git[len + 1] = '\0';
>>> + add_exclude(basename, "", 0, el, 0);
Hmm.. I overlooked this bit before. So if $GIT_DIR is /something/foo,
we set to ignore "foo/". Because we know n_git must be part of
(normalized) get_git_work_tree() at this point, could we path n_git +
strlen(get_git_work_tree()) to add_exclude() instead of basename? Full
path makes sure we don't accidentally exclude too much.
The case when $GIT_DIR points to a _file_ seems uncovered.
setup_git_directory() will transform the file to the directory
internally and we never know the .git file's path (so we can't exclude
it). So people could accidentally add the .git file in, then remove it
from from work tree and suddenly the work tree becomes repo-less. It's
not as bad as .git _directory_ because we don't lose valuable data. I
don't know if you want to cover this too.
--
Duy
--
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