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

Re: [PATCH 0/2] midx: prevent against racily disappearing packs

From
Jeff King <peff@peff.net>
Date
Nov 26, 2020, 01:27 UTC
Message-ID
<X78EgOPAVg1vQRo9@coredump.intra.peff.net>
In-Reply-To
<X77/LEwWlz9fjbYM@nand.local>
On Wed, Nov 25, 2020 at 08:04:44PM -0500, Taylor Blau wrote:
Show 13 quoted lines
> > So we could perhaps BUG() in find_revindex_entry() instead of returning
> > NULL. A quick segfault accomplishes mostly the same thing, though the
> > BUG() could distinguish the two cases more clearly.
> 
> Yeah, a find_revindex_entry() that returns NULL means that the caller is
> probably dead in the water.
> 
> FWIW, this function gets touched by a series that I'm working on here:
> [1]. There, I think "returning NULL" is equivalent to "returning -1",
> and the problem exists there, too.
> 
> We could return a different negative number, call BUG(), or do nothing
> other than what's written. I don't have any strong feelings, though.

Yeah, I was suggesting _never_ returning a failure, and just hitting a BUG() within the function. So it does not matter then how you represent the error return type, because there isn't one. :)

Looking over the callers, there are actually a few that check the return value and handle it sanely. I suspect they can never trigger in practice, given that the other callers would all segfault, and we have not seen any reports in the 15 years during which that has been the case. But perhaps some of them can be triggered by bogus pack data that nobody has ever run into in the real world.

At any rate, I am content to ignore it until somebody feels like digging into each caller. A BUG() is only marginally better than an immediate segfault anyway, and I'd prefer not to disrupt more substantive improvements in the area.

-Peff
Previous: Taylor Blau
Message 9 of 9 in “midx: prevent against racily disappearing packs”
  1. 0/2 midx: prevent against racily disappearing packsTaylor Blau, Nov 25, 2020
  2. 1/2 packfile.c: protect against disappearing indexesTaylor Blau, Nov 25, 2020
  3. Junio C HamanoNov 25, 2020
  4. 2/2 midx.c: protect against disappearing packsTaylor Blau, Nov 25, 2020
  5. Junio C HamanoNov 25, 2020
  6. Junio C HamanoNov 25, 2020
  7. Jeff KingNov 26, 2020
  8. Taylor BlauNov 26, 2020
  9. Jeff KingNov 26, 2020

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.