Re: [msysGit] [PATCH] fix deletion of .git/objects sub-directories in git-prune/repack

Junio C Hamano [off-list ref] wrote on 06.03.2012 21:19:06:

Johannes Sixt [off-list ref] writes:

quoted

quoted

diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index f9463de..a834417 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c

@@ -36,7 +36,6 @@ static void prune_dir(int i, DIR *dir, char 

*pathname, 

quoted

quoted

int len, int opts)
                display_progress(progress, i + 1);
        }
        pathname[len] = 0;
-       rmdir(pathname);

After moving the rmdir() away from prune_dir(), the truncation of the
pathname does not logically belong here anymore. It should be moved 

with

quoted

the rmdir(). Looks good otherwise.

I agree that it is better to have the NUL termination close to where
it actually matters.

The pathname is extended in prune_dir, so I think it should be reset there
as well; moving it to prune_packed_objects would be quite obscure:

d = opendir(pathname);
prune_dir(d, pathname);
pathname[len] = 0;
rmdir(pathname);


OT: While looking at the code I just stumbled across this immediately
above the patch (prune-packed.c line 32ff):

                memcpy(pathname + len, de->d_name, 38);
                if (opts & DRY_RUN)
                        printf("rm -f %s\n", pathname);
                else
                        unlink_or_warn(pathname);

Shouldn't this be memcpy(..., 39) (i.e. including '\0')?

Do you want me to take it after locally fixing it up, or do you
prefer to feed this as part of msysgit related updates to me later?

The msysgit queue is quite long, and I think it makes sense to fast-track 
this one.