Re: [PATCH] dir: remove dead code

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

 



Am 08.09.2013 16:42, schrieb Ramkumar Ramachandra:
On Sun, Sep 8, 2013 at 6:00 PM, René Scharfe <l.s.r@xxxxxx
<mailto:l.s.r@xxxxxx>> wrote:

    Am 08.09.2013 08:09, schrieb Ramkumar Ramachandra:

        Remove dead code around remove_dir_recursively().


    This basically reverts ae2f203e (clean: preserve nested git worktree
    in subdirectories).  t7300 still seems to pass, though.  I wonder why.


t7300 has nothing to do with ae2f203e.

ae2f203e modified t/t7300-clean.sh.


           dir.c | 21 ++++-----------------
           1 file changed, 4 insertions(+), 17 deletions(-)

        diff --git a/dir.c b/dir.c
        index 910bfcd..2b31241 100644
        --- a/dir.c
        +++ b/dir.c
        @@ -1464,11 +1464,11 @@ int is_empty_dir(const char *path)
                 return ret;
           }

        -static int remove_dir_recurse(struct strbuf *path, int flag,
        int *kept_up)
        +int remove_dir_recursively(struct strbuf *path, int flag)
           {
                 DIR *dir;
                 struct dirent *e;
        -       int ret = 0, original_len = path->len, len, kept_down = 0;
        +       int ret = 0, original_len = path->len, len;
                 int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
                 int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
                 unsigned char submodule_head[20];
        @@ -1476,8 +1476,6 @@ static int remove_dir_recurse(struct
        strbuf *path, int flag, int *kept_up)
                 if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
                     !resolve_gitlink_ref(path->__buf, "HEAD",
        submodule_head)) {
                         /* Do not descend and nuke a nested git work
        tree. */
        -               if (kept_up)
        -                       *kept_up = 1;
                         return 0;
                 }

        @@ -1504,7 +1502,7 @@ static int remove_dir_recurse(struct
        strbuf *path, int flag, int *kept_up)
                         if (lstat(path->buf, &st))
                                 ; /* fall thru */
                         else if (S_ISDIR(st.st_mode)) {
        -                       if (!remove_dir_recurse(path, flag,
        &kept_down))
        +                       if (!remove_dir_recursively(path, flag))


    kept_down could have been set to 1 here...


Not possible.

Why?

I guess the answer is that kept_down could have only been set if the flag REMOVE_DIR_KEEP_NESTED_GIT is given, which only git clean uses, which in turn has its own implementation of remove_dir_recursively() named remove_dirs() since f538a91e (git-clean: Display more accurate delete messages).

That probably means you can remove even more code from remove_dir_recursively().


                                         continue; /* happy */
                         } else if (!only_empty && !unlink(path->buf))
                                 continue; /* happy, too */
        @@ -1516,22 +1514,11 @@ static int remove_dir_recurse(struct
        strbuf *path, int flag, int *kept_up)
                 closedir(dir);

                 strbuf_setlen(path, original_len);
        -       if (!ret && !keep_toplevel && !kept_down)
        +       if (!ret && !keep_toplevel)
                         ret = rmdir(path->buf);


    ... and would have prevented the rmdir() call here.

    Is the removed code really dead?  And if not, why does t7300 still pass?


Yes, it clearly is. Shared secret.

What is the secret and who shares it?

Thanks,
René

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