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

Re: [PATCH 35/53] Convert the verify_pack callback to struct object_id

From
Jeff King <peff@peff.net>
Date
Apr 27, 2017, 05:52 UTC
Message-ID
<20170427055209.ock533opgzans6ew@sigill.intra.peff.net>
In-Reply-To
<20170423213453.253425-36-sandals@crustytoothpaste.net>
On Sun, Apr 23, 2017 at 09:34:35PM +0000, brian m. carlson wrote:
> Make the verify_pack_callback take a pointer to struct object_id.
That sounds good.
Show 5 quoted lines
> Use a
> struct object_id to hold the pack checksum, even though it is not
> strictly an object ID.  Doing so ensures resilience against future hash
> size changes, and allows us to remove hard-coded assumptions about how
> big the buffer needs to be.

But this part seems questionable to me. Sure, we may change the pack checksum in the future. But there is a reasonable chance that it won't follow the same rules for selecting a hash as object_id. And even if it does, calling it object_id just seems misleading.

What's the gain in converting it here? I know we want to get rid of the bare "20", but we could switch it out for GIT_SHA1_RAWSZ. I suspect you prefer in the long run to get rid of even those GIT_SHA1_RAWSZ defines, though. Could we define a new struct csumfile_hash, csumfile_cmp, etc (and arguably change the name of "struct sha1file" and friends). They'd be boring wrappers around sha1 now, but I think the endgame will involve us being able to read multiple versions of packs, with distinct checksum algorithms.

Show 8 quoted lines
> Also, use a union to convert the pointer from nth_packed_object_sha1 to
> to a pointer to struct object_id.  This behavior is compatible with GCC
> and clang and explicitly sanctioned by C11.  The alternatives are to
> just perform a cast, which would run afoul of strict aliasing rules, but
> should just work, and changing the pointer into an instance of struct
> object_id and copying the value.  The latter operation could seriously
> bloat memory usage on fsck, which already uses a lot of memory on some
> repositories.
Nasty, but it's probably the least-bad option.
-Peff
Previous: Stefan BellerNext: brian m. carlson
Message 42 of 67 in “object_id part 8”
  1. 00/53 object_id part 8brian m. carlson, Apr 23, 2017
  2. 02/53 Clean up outstanding object_id transforms.brian m. carlson, Apr 23, 2017
  3. 01/53 fetch-pack: convert to struct object_idbrian m. carlson, Apr 23, 2017
  4. 03/53 Convert struct cache_tree to use struct object_idbrian m. carlson, Apr 23, 2017
  5. 04/53 builtin/name-rev: convert to struct object_idbrian m. carlson, Apr 23, 2017
  6. 05/53 builtin/prune: convert to struct object_idbrian m. carlson, Apr 23, 2017
  7. 06/53 bundle: convert to struct object_idbrian m. carlson, Apr 23, 2017
  8. 08/53 builtin/blame: convert static function to struct object_idbrian m. carlson, Apr 23, 2017
  9. 07/53 branch: convert to struct object_idbrian m. carlson, Apr 23, 2017
  10. 10/53 fast-import: convert internal structs to struct object_idbrian m. carlson, Apr 23, 2017
  11. 09/53 builtin/rev-parse: convert to struct object_idbrian m. carlson, Apr 23, 2017
  12. 12/53 submodule: convert merge_submodule to use struct object_idbrian m. carlson, Apr 23, 2017
  13. 13/53 notes-cache: convert to struct object_idbrian m. carlson, Apr 23, 2017
  14. Stefan BellerApr 24, 2017
  15. 11/53 fast-import: convert to struct object_idbrian m. carlson, Apr 23, 2017
  16. Stefan BellerApr 24, 2017
  17. brian m. carlsonApr 24, 2017
  18. 14/53 parse-options-cb: convert to struct object_idbrian m. carlson, Apr 23, 2017
  19. 17/53 tag: convert parse_tag_buffer to struct object_idbrian m. carlson, Apr 23, 2017
  20. 15/53 reflog_expire: convert to struct object_idbrian m. carlson, Apr 23, 2017
  21. 16/53 builtin/verify-commit: convert to struct object_idbrian m. carlson, Apr 23, 2017
  22. 19/53 notes-utils: convert internals to struct object_idbrian m. carlson, Apr 23, 2017
  23. 18/53 http-push: convert some static functions to struct object_idbrian m. carlson, Apr 23, 2017
  24. 22/53 sequencer: convert some functions to struct object_idbrian m. carlson, Apr 23, 2017
  25. 20/53 revision: convert prepare_show_merge to struct object_idbrian m. carlson, Apr 23, 2017
  26. 21/53 shallow: convert shallow registration functions to object_idbrian m. carlson, Apr 23, 2017
  27. 23/53 builtin/tag: convert to struct object_idbrian m. carlson, Apr 23, 2017
  28. 24/53 Convert remaining callers of lookup_commit_reference* to object_idbrian m. carlson, Apr 23, 2017
  29. 26/53 pack: convert struct pack_idx_entry to struct object_idbrian m. carlson, Apr 23, 2017
  30. 25/53 Convert lookup_commit* to struct object_idbrian m. carlson, Apr 23, 2017
  31. 27/53 builtin/unpack-objects: convert to struct object_idbrian m. carlson, Apr 23, 2017
  32. 28/53 Convert remaining callers of lookup_blob to object_idbrian m. carlson, Apr 23, 2017
  33. Stefan BellerApr 24, 2017
  34. 29/53 Convert lookup_blob to struct object_idbrian m. carlson, Apr 23, 2017
  35. 30/53 tree: convert read_tree_1 to use struct object_id internallybrian m. carlson, Apr 23, 2017
  36. 31/53 builtin/reflog: convert tree_is_complete to take struct object_idbrian m. carlson, Apr 23, 2017
  37. 34/53 Convert lookup_tag to struct object_idbrian m. carlson, Apr 23, 2017
  38. 32/53 Convert lookup_tree to struct object_idbrian m. carlson, Apr 23, 2017
  39. 33/53 log-tree: convert to struct object_idbrian m. carlson, Apr 23, 2017
  40. 35/53 Convert the verify_pack callback to struct object_idbrian m. carlson, Apr 23, 2017
  41. Stefan BellerApr 26, 2017
  42. Jeff KingApr 27, 2017
  43. brian m. carlsonApr 28, 2017
  44. Jeff KingApr 28, 2017
  45. 36/53 Convert struct ref_array_item to struct object_idbrian m. carlson, Apr 23, 2017
  46. 37/53 ref-filter: convert some static functions to struct object_idbrian m. carlson, Apr 23, 2017
  47. 38/53 refs: convert struct ref_update to use struct object_idbrian m. carlson, Apr 23, 2017
  48. Michael HaggertyApr 27, 2017
  49. 39/53 refs/files-backend: convert many internals to struct object_idbrian m. carlson, Apr 23, 2017
  50. Michael HaggertyApr 27, 2017
  51. 40/53 http-push: convert process_ls_object and descendants to object_idbrian m. carlson, Apr 23, 2017
  52. 41/53 revision: rename add_pending_sha1 to add_pending_oidbrian m. carlson, Apr 23, 2017
  53. 43/53 upload-pack: convert remaining parse_object callers to object_idbrian m. carlson, Apr 23, 2017
  54. 46/53 builtin/ls-files: convert overlay_tree_on_cache to object_idbrian m. carlson, Apr 23, 2017
  55. 45/53 builtin/read-tree: convert to struct object_idbrian m. carlson, Apr 23, 2017
  56. 47/53 sequencer: convert fast_forward_to to struct object_idbrian m. carlson, Apr 23, 2017
  57. 51/53 sequencer: convert do_recursive_merge to struct object_idbrian m. carlson, Apr 23, 2017
  58. 50/53 diff-lib: convert do_diff_cache to struct object_idbrian m. carlson, Apr 23, 2017
  59. 48/53 merge: convert checkout_fast_forward to struct object_idbrian m. carlson, Apr 23, 2017
  60. 49/53 builtin/ls-tree: convert to struct object_idbrian m. carlson, Apr 23, 2017
  61. 44/53 sha1_name: convert internals of peel_onion to object_idbrian m. carlson, Apr 23, 2017
  62. 42/53 revision: convert remaining parse_object callers to object_idbrian m. carlson, Apr 23, 2017
  63. 52/53 tree: convert parse_tree_indirect to struct object_idbrian m. carlson, Apr 23, 2017
  64. 53/53 object: convert parse_object* to take struct object_idbrian m. carlson, Apr 23, 2017
  65. Stefan BellerApr 27, 2017
  66. Junio C HamanoApr 24, 2017
  67. Brandon WilliamsApr 24, 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.