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

Re: [PATCH 04/31] pack-bitmap: replace sha1_to_hex

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Feb 13, 2019, 00:00 UTC
Message-ID
<20190213000007.GG684736@genre.crustytoothpaste.net>
In-Reply-To
<20190212063749.GC21872@sigill.intra.peff.net>
On Tue, Feb 12, 2019 at 01:37:49AM -0500, Jeff King wrote:
Show 9 quoted lines
> On Tue, Feb 12, 2019 at 01:22:29AM +0000, brian m. carlson wrote:
> > -static uint32_t find_object_pos(const unsigned char *sha1)
> > +static uint32_t find_object_pos(const unsigned char *hash)
> 
> Isn't this really just a "struct object_id"? Why don't we want to use
> that here?
> 
> I suspect it may be partially because our khash is storing raw sha1s.
> But shouldn't we also be converted that to store object_ids?

I think probably there are some more places that we could convert here. There may have been one or two places that weren't convertible because we ended up passing data from some sort of buffer around. I'll take another look.

Show 13 quoted lines
> In particular:
> 
> > @@ -181,7 +181,7 @@ static struct stored_bitmap *store_bitmap(struct bitmap_index *index,
> >  	stored->root = root;
> >  	stored->xor = xor_with;
> >  	stored->flags = flags;
> > -	oidread(&stored->oid, sha1);
> > +	oidread(&stored->oid, hash);
> >  
> >  	hash_pos = kh_put_sha1(index->bitmaps, stored->oid.hash, &ret);
> 
> This last line (which is actually from the previous patch) is going to
> always truncate the stored data to 20 bytes, isn't it?

No, I don't think it does. The _sha1 variant stores pointers to unsigned char, while the _oid variant stores the entire struct object_id (not just a pointer to it). We don't care how much data the pointer points to.

> I think we need to define a kh_oid. We have the "set" version already in
> oidset.[ch]; I think we need to make that more public.

I wrote this quite a bit before that code came in, which is probably why I didn't do that originally. I seem to recall last time I looked at this that there was some reason that hoisting this didn't work as I expected due to header include order, but I'll take a look and see if I can figure out a way to do this.

-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
Previous: Jeff KingNext: Jeff King
Message 9 of 56 in “Hash function transition part 16”
  1. 00/31 Hash function transition part 16brian m. carlson, Feb 12, 2019
  2. 01/31 t/lib-submodule-update: use appropriate length constantbrian m. carlson, Feb 12, 2019
  3. 03/31 pack-bitmap: convert struct stored_bitmap to object_idbrian m. carlson, Feb 12, 2019
  4. 02/31 pack-bitmap: make bitmap header handling hash agnosticbrian m. carlson, Feb 12, 2019
  5. 05/31 pack-bitmap: switch hard-coded constants to the_hash_algobrian m. carlson, Feb 12, 2019
  6. Ævar Arnfjörð BjarmasonFeb 12, 2019
  7. 04/31 pack-bitmap: replace sha1_to_hexbrian m. carlson, Feb 12, 2019
  8. Jeff KingFeb 12, 2019
  9. brian m. carlsonFeb 13, 2019
  10. Jeff KingFeb 14, 2019
  11. 08/31 notes: make hash size independentbrian m. carlson, Feb 12, 2019
  12. Eric SunshineFeb 12, 2019
  13. brian m. carlsonFeb 12, 2019
  14. 09/31 notes: replace sha1_to_hexbrian m. carlson, Feb 12, 2019
  15. 11/31 builtin/name-rev: make hash-size independentbrian m. carlson, Feb 12, 2019
  16. 06/31 submodule: avoid hard-coded constantsbrian m. carlson, Feb 12, 2019
  17. 10/31 object-store: rename and expand packed_git's sha1 memberbrian m. carlson, Feb 12, 2019
  18. Eric SunshineFeb 12, 2019
  19. brian m. carlsonFeb 14, 2019
  20. 14/31 builtin/am: make hash size independentbrian m. carlson, Feb 12, 2019
  21. 15/31 builtin/pull: make hash-size independentbrian m. carlson, Feb 12, 2019
  22. Eric SunshineFeb 12, 2019
  23. 13/31 fast-import: replace sha1_to_hexbrian m. carlson, Feb 12, 2019
  24. 17/31 http-backend: allow 64-character hex namesbrian m. carlson, Feb 12, 2019
  25. 16/31 http-push: convert to use the_hash_algobrian m. carlson, Feb 12, 2019
  26. 12/31 fast-import: make hash-size independentbrian m. carlson, Feb 12, 2019
  27. Eric SunshineFeb 12, 2019
  28. brian m. carlsonFeb 12, 2019
  29. 18/31 http-push: remove remaining uses of sha1_to_hexbrian m. carlson, Feb 12, 2019
  30. 20/31 http: replace hard-coded constant with the_hash_algobrian m. carlson, Feb 12, 2019
  31. 22/31 http: replace sha1_to_hexbrian m. carlson, Feb 12, 2019
  32. 21/31 http: compute hash of downloaded objects using the_hash_algobrian m. carlson, Feb 12, 2019
  33. 23/31 remote-curl: make hash size independentbrian m. carlson, Feb 12, 2019
  34. Ævar Arnfjörð BjarmasonFeb 12, 2019
  35. 19/31 http-walker: replace sha1_to_hexbrian m. carlson, Feb 12, 2019
  36. Eric SunshineFeb 12, 2019
  37. 29/31 read-cache: read data in a hash-independent waybrian m. carlson, Feb 12, 2019
  38. 28/31 dir: make untracked cache extension hash size independentbrian m. carlson, Feb 12, 2019
  39. Ævar Arnfjörð BjarmasonFeb 12, 2019
  40. brian m. carlsonFeb 13, 2019
  41. 25/31 archive: convert struct archiver_args to object_idbrian m. carlson, Feb 12, 2019
  42. 26/31 refspec: make hash size independentbrian m. carlson, Feb 12, 2019
  43. 31/31 gitweb: make hash size independentbrian m. carlson, Feb 12, 2019
  44. Ævar Arnfjörð BjarmasonFeb 12, 2019
  45. 30/31 Git.pm: make hash size independentbrian m. carlson, Feb 12, 2019
  46. Ævar Arnfjörð BjarmasonFeb 12, 2019
  47. brian m. carlsonFeb 18, 2019
  48. Ævar Arnfjörð BjarmasonFeb 18, 2019
  49. 24/31 archive-tar: make hash size independentbrian m. carlson, Feb 12, 2019
  50. René ScharfeFeb 12, 2019
  51. René ScharfeFeb 12, 2019
  52. brian m. carlsonFeb 13, 2019
  53. 27/31 builtin/difftool: use parse_oid_hexbrian m. carlson, Feb 12, 2019
  54. Eric SunshineFeb 12, 2019
  55. 07/31 notes-merge: switch to use the_hash_algobrian m. carlson, Feb 12, 2019
  56. Ævar Arnfjörð BjarmasonFeb 12, 2019

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.