Re: [PATCH 01/12] for_each_alternate_ref: handle failure from real_pathdup()
Jeff King <peff@peff.net> writes:
Show 21 quoted lines
> In older versions of git, if real_path() failed to resolve
> the alternate object store path, we would die() with an
> error. However, since 4ac9006f8 (real_path: have callers use
> real_pathdup and strbuf_realpath, 2016-12-12) we use the
> real_pathdup() function, which may return NULL. Since we
> don't check the return value, we can segfault.
>
> This is hard to trigger in practice, since we check that the
> path is accessible before creating the alternate_object_database
> struct. But it could be removed racily, or we could see a
> transient filesystem error.
>
> We could restore the original behavior by switching back to
> xstrdup(real_path()). However, dying is probably not the
> best option here. This whole function is best-effort
> already; there might not even be a repository around the
> shared objects at all. And if the alternate store has gone
> away, there are no objects to show.
>
> So let's just quietly return, as we would if we failed to
> open "refs/", or if upload-pack failed to start, etc.
That's sensible, methinks.
Show 19 quoted lines
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> transport.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/transport.c b/transport.c
> index c86ba2eb8..74d0e45bd 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1215,6 +1215,8 @@ static int refs_from_alternate_cb(struct alternate_object_database *e,
> struct alternate_refs_data *cb = data;
>
> other = real_pathdup(e->path);
> + if (!other)
> + return 0;
> len = strlen(other);
>
> while (other[len-1] == '/')