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

Re: [PATCH 4/4] shallow.c: remove useless test

From
Duy Nguyen <pclouds@gmail.com>
Date
Dec 5, 2016, 12:00 UTC
Message-ID
<CACsJy8DSw_EXojKYXvkkqkbd3fsQJ=hhAb0GCfMYRzK2S3d-3Q@mail.gmail.com>
In-Reply-To
<20161203052422.hhaj3idboo6r6dz5@sigill.intra.peff.net>
On Sat, Dec 3, 2016 at 12:24 PM, Jeff King <peff@peff.net> wrote:
Show 7 quoted lines
> On Fri, Dec 02, 2016 at 09:31:04PM +0100, Rasmus Villemoes wrote:
>
>> It seems to be odd to do x=y if x==y. Maybe there's a bug somewhere near
>> this, but as is this is somewhat confusing.
>
> Yeah, this code is definitely wrong, but I'm not sure what it's trying
> to do. This is the first time I've looked at it.

I'm sorry I don't know why it's there either :( The first version that has this was v3 [1] which still uses "util" pointdf instead of the commit slab but the logic does not differ much.

This is the place when we "paint" the parent commit with the same bitmap as the child I mentioned earlier (though I think I mentioned it backward). You see similar code in the same loop just a bit earlier: if the commit has not been painted, it gets a new bitmap, otherwise new refs are OR'd to its bitmap. It's the OR part (when the bitmaps pointed by *p_refs and *refs differ, it's not just about pointer comparison) that's probably missing here.

But it looks like we can safely delete the " || *p_refs == *refs" part because the commit in question is inserted back to the commit list "head" and revisited in the next iteration. If its bitmap is different from the child's, then in the next iteration it should hit the "if (memcmp(tmp, *refs, bitmap_size))" line above, in the same loop, then the new bit will be added. If it's marked UNINTERESTING though, that won't happen. I'll need more time to stare at this code...

[1] http://public-inbox.org/git/1385351754-9954-9-git-send-email-pclouds@gmail.com/
-- 
Duy
Previous: Jeff KingNext: Jeff King
Message 8 of 20 in “shallow.c: make paint_alloc slightly more robust”
  1. 1/4 shallow.c: make paint_alloc slightly more robustRasmus Villemoes, Dec 2, 2016
  2. 2/4 shallow.c: avoid theoretical pointer wrap-aroundRasmus Villemoes, Dec 2, 2016
  3. Jeff KingDec 3, 2016
  4. 3/4 shallow.c: bit manipulation tweaksRasmus Villemoes, Dec 2, 2016
  5. Jeff KingDec 3, 2016
  6. 4/4 shallow.c: remove useless testRasmus Villemoes, Dec 2, 2016
  7. Jeff KingDec 3, 2016
  8. Duy NguyenDec 5, 2016
  9. Jeff KingDec 3, 2016
  10. Duy NguyenDec 5, 2016
  11. 0/6 shallow.c improvementsNguyễn Thái Ngọc Duy, Dec 6, 2016
  12. 1/6 shallow.c: rename fields in paint_info to better express their purposesNguyễn Thái Ngọc Duy, Dec 6, 2016
  13. 5/6 shallow.c: bit manipulation tweaksNguyễn Thái Ngọc Duy, Dec 6, 2016
  14. 4/6 shallow.c: avoid theoretical pointer wrap-aroundNguyễn Thái Ngọc Duy, Dec 6, 2016
  15. 6/6 shallow.c: remove useless codeNguyễn Thái Ngọc Duy, Dec 6, 2016
  16. 3/6 shallow.c: make paint_alloc slightly more robustNguyễn Thái Ngọc Duy, Dec 6, 2016
  17. 2/6 shallow.c: stop abusing COMMIT_SLAB_SIZE for paint_info's memory poolsNguyễn Thái Ngọc Duy, Dec 6, 2016
  18. Jeff KingDec 6, 2016
  19. Duy NguyenDec 6, 2016
  20. Junio C HamanoDec 7, 2016

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.