Re: [PATCH] refs: make sure we never pass NULL to hashcpy

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 8, 2017, 17:15 UTC
Message-ID: <xmqqfubxyuuw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <ab0c4064-6eaa-c298-a26d-58fc83e4574f@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> So `ref_transaction_update()` *does* need to set or clear the `HAVE_NEW`
> and `HAVE_OLD` bits as I sketched, to impedance-match between the two
> conventions.

OK, so ignoring HAVE_NEW/HAVE_OLD bits that the callers of ref_transaction_update() may set in flags, and having ref_transaction_update() compute these bits based on new/old_sha1 pointers from scratch, would be the right thing to do.

IOW

	flags &= ~(REF_HAVE_NEW|REF_HAVE_OLD);
	if (new_sha1)
		flags |= REF_HAVE_NEW;
	if (old_sha1)
		flags |= REF_HAVE_OLD;

and your earlier "Does the warning go away if you change the line to" does essentially the same thing.

> It's a shame how much time we've wasted discussing this. Maybe the code
> is trying to be too clever/efficient and needs a rethink.

It might be the case, but I do not know what to blame is "the two conventions", an over-eager compiler, or a confused commenter on the thread (that's me), though ;-).