On Wed, Oct 5, 2016 at 7:29 AM, Jeff King <peff@xxxxxxxx> 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 <peff@xxxxxxxx>
> ---
> 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