On Wed, Oct 5, 2016 at 7:29 AM, Jeff King <[email protected]> wrote: > When we are copying the alternates from the source > repository, if we find a relative path that is too deep for > the source (e.g., "../../../objects" from "/repo.git/objects"), > then normalize_path_copy will report an error and leave > trash in the buffer, which we will add to our new alternates > file. Instead, let's detect the error, print a warning, and > skip copying that alternate. > > There's no need to die. The relative path is probably just > broken cruft in the source repo. If it turns out to have > been important for accessing some objects, we rely on other > parts of the clone to detect that, just as they would with a > missing object in the source repo itself (though note that > clones with "-s" are inherently local, which may do fewer > object-quality checks in the first place).
This explanation and the implementation make sense. Thanks! > > Signed-off-by: Jeff King <[email protected]> > --- > Noticed by coverity; I saw them, too and wanted to start preparing a patch, but I cannot quite compete with your speed here. ;) > the recent alternates cleanups mean that all of the > other calls to normalize_path_copy() are now checked, so it realized > this one was an oddball and probably an error (I actually looked for > others with `grep` when doing that series, but somehow missed this one; > hooray for static analysis). The fix is independent of that series. > > builtin/clone.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/builtin/clone.c b/builtin/clone.c > index fb75f7e..6cf3b54 100644 > --- a/builtin/clone.c > +++ b/builtin/clone.c > @@ -345,8 +345,11 @@ static void copy_alternates(struct strbuf *src, struct > strbuf *dst, > continue; > } > abs_path = mkpathdup("%s/objects/%s", src_repo, line.buf); > - normalize_path_copy(abs_path, abs_path); > - add_to_alternates_file(abs_path); > + if (!normalize_path_copy(abs_path, abs_path)) > + add_to_alternates_file(abs_path); > + else > + warning("skipping invalid relative alternate: %s/%s", > + src_repo, line.buf); > free(abs_path); > } > strbuf_release(&line); > -- > 2.10.1.506.g904834d
