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

From: Jeff King <peff@peff.net>
Date: Aug 19, 2016, 20:58 UTC
Message-ID: <20160819205806.vxzzgphqf4lgdor7@sigill.intra.peff.net>
In-Reply-To: <xmqqfuq01oww.fsf@gitster.mtv.corp.google.com>

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

Show 15 quoted lines

> 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