git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 01/12] for_each_alternate_ref: handle failure from real_pathdup()

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 25, 2017, 18:26 UTC
Message-ID
<xmqqziifatcz.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170124003827.l2rimgitsyxsvtly@sigill.intra.peff.net>
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] == '/')
Previous: Jeff KingNext: Jeff King
Message 3 of 48 in “reducing resource usage of for_each_alternate_ref”
  1. 0/12 reducing resource usage of for_each_alternate_refJeff King, Jan 24, 2017
  2. 01/12 for_each_alternate_ref: handle failure from real_pathdup()Jeff King, Jan 24, 2017
  3. Junio C HamanoJan 25, 2017
  4. 02/12 for_each_alternate_ref: stop trimming trailing slashesJeff King, Jan 24, 2017
  5. 03/12 for_each_alternate_ref: use strbuf for path allocationJeff King, Jan 24, 2017
  6. Junio C HamanoJan 25, 2017
  7. Jeff KingJan 25, 2017
  8. 04/12 for_each_alternate_ref: pass name/oid instead of ref structJeff King, Jan 24, 2017
  9. 05/12 for_each_alternate_ref: replace transport code with for-each-refJeff King, Jan 24, 2017
  10. Junio C HamanoJan 25, 2017
  11. 06/12 clone: disable save_commit_bufferJeff King, Jan 24, 2017
  12. Junio C HamanoJan 25, 2017
  13. Jeff KingJan 25, 2017
  14. Jeff KingJan 25, 2017
  15. Jeff KingJan 25, 2017
  16. 07/12 fetch-pack: cache results of for_each_alternate_refJeff King, Jan 24, 2017
  17. Junio C HamanoJan 25, 2017
  18. Jeff KingJan 25, 2017
  19. 08/12 add oidset APIJeff King, Jan 24, 2017
  20. Ramsay JonesJan 24, 2017
  21. Jeff KingJan 24, 2017
  22. 10/12 receive-pack: fix misleading namespace/.have commentJeff King, Jan 24, 2017
  23. 09/12 receive-pack: use oidset to de-duplicate .have linesJeff King, Jan 24, 2017
  24. Junio C HamanoJan 25, 2017
  25. Jeff KingJan 25, 2017
  26. 12/12 receive-pack: avoid duplicates between our refs and alternatesJeff King, Jan 24, 2017
  27. Junio C HamanoJan 25, 2017
  28. Jeff KingJan 25, 2017
  29. 11/12 receive-pack: treat namespace .have lines like alternatesJeff King, Jan 24, 2017
  30. Junio C HamanoJan 25, 2017
  31. Jeff KingJan 25, 2017
  32. Lukas FleischerJan 27, 2017
  33. Jeff KingJan 27, 2017
  34. Junio C HamanoJan 27, 2017
  35. Brandon WilliamsJan 24, 2017
  36. Jeff KingJan 24, 2017
  37. 0/11 reducing resource usage of for_each_alternate_refJeff King, Feb 8, 2017
  38. 01/11 for_each_alternate_ref: handle failure from real_pathdup()Jeff King, Feb 8, 2017
  39. 02/11 for_each_alternate_ref: stop trimming trailing slashesJeff King, Feb 8, 2017
  40. 04/11 for_each_alternate_ref: pass name/oid instead of ref structJeff King, Feb 8, 2017
  41. 03/11 for_each_alternate_ref: use strbuf for path allocationJeff King, Feb 8, 2017
  42. 05/11 for_each_alternate_ref: replace transport code with for-each-refJeff King, Feb 8, 2017
  43. 08/11 receive-pack: use oidset to de-duplicate .have linesJeff King, Feb 8, 2017
  44. 07/11 add oidset APIJeff King, Feb 8, 2017
  45. 06/11 fetch-pack: cache results of for_each_alternate_refJeff King, Feb 8, 2017
  46. 09/11 receive-pack: fix misleading namespace/.have commentJeff King, Feb 8, 2017
  47. 10/11 receive-pack: treat namespace .have lines like alternatesJeff King, Feb 8, 2017
  48. 11/11 receive-pack: avoid duplicates between our refs and alternatesJeff King, Feb 8, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.