Re: [PATCH v4] Add an explicit GIT_DIR to the list of excludes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 28, 2014 at 5:36 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Tue, May 27, 2014 at 10:56 AM, Pasha Bolokhov
> <pasha.bolokhov@xxxxxxxxx> wrote:
>> @@ -1588,6 +1588,38 @@ void setup_standard_excludes(struct dir_struct *dir)
>>  {
>>         const char *path;
>>         char *xdg_path;
>> +       const char *r_git, *gitdir = get_git_dir();
>> +       char *n_git, *basename;
>> +       int len, i;
>> +
>> +       /*
>> +        * Add git directory to the ignores; do this only if
>> +        * GIT_DIR does not end with "/.git"
>> +        */
>> +       r_git = real_path(absolute_path(gitdir));
>> +       n_git = xmalloc(strlen(r_git) + 1 + 1);
>> +       normalize_path_copy(n_git, r_git);
>> +
>> +       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. 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--) ;
        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);
>> +               }
>> +       }
>> +       free(n_git);
>
> All this add-only code makes me think it may be nice to make it a
> separate function. A good function name could replace the comment near
> the beginning of the block.

Reasonable
I'll send the all-updated patch including doc when ready

> --
> 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]