Re: [PATCH v8 7/8] cache: add empty_tree_oid object

[Jeff King <peff@peff.net>]

On Fri, Aug 19, 2016 at 01:31:27PM -0700, Junio C Hamano wrote:

> Jacob Keller <jacob.e.keller@intel.com> writes:
> 
> > From: Jacob Keller <jacob.keller@gmail.com>
> >
> > Add an empty_tree_oid object which can be used in place of
> > EMPTY_TREE_SHA1_BIN_LITERAL for code which is being converted to struct
> > object_id.
> 
> How widely do you envision the users of this symbol would be spread
> across the entire codebase?  I am debating myself if we need a
> singleton in-core copy like this (if we end up referring to it from
> everywhere), or we only need one level higher abstraction,
> e.g. "is_empty_tree_oid()" helper (in which case I do not think we
> even need a singleton; just imitate how is_empty_blob_sha1() is
> implemented).

I suspect we need more than just the "is_empty" query. At least for the
blob case, we do hashcpy() it into a struct (which should eventually
become oidcpy). The empty-tree case even more so, as we pass it to
random functions like lookup_tree().

Our EMPTY_TREE_SHA1_BIN_LITERAL effectively ends up as a singleton
in-core, too; it's just handled transparently by the compiler, since
it's a literal. This effectively gives us _two_ singletons. We could do:

  const struct object_id empty_blob_oid = {
	  "\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b"
	  "\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
  };
  #define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)

It's possible the use of an actual string literal lets the compiler do
more optimizations, but I'd doubt it matters in practice. Probably it is
just sticking that literal somewhere in BSS and filling in the pointer
to it.

-Peff