Junio C Hamano <gitster@xxxxxxxxx> wrote on 06.03.2012 21:19:06:
> Johannes Sixt <j6t@xxxxxxxx> writes:
>
> >> 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,
> >> 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
> > 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.
--
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