Re: [PATCH v4] Replace memcpy with hashcpy when dealing hash copy globally

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2014, 19:32 UTC
Message-ID: <xmqqpplzxgv4.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1394097084-3129-1-git-send-email-sunheehnus@gmail.com>

Sun He <sunheehnus@gmail.com> writes:

> hashcpy() can keep the abstraction of "object name" behind it.

Do we really want to change the phrasing you took the above from to say "*can* keep"?

Providing the "object name" abstraction is the whole point of the function, so of course it can keep it, but that goes without saying---it was the sole reason why it was invented in the first place.

> Use it instead of memcpy() with hard-coded 20-byte length when
> moving object names between pieces of memory.
> We can benefit from it, when we switch to another hash algorithm,
> eg. MD5, by just fixing the hashcpy().

"fix" can be used in two scenarios, I think. Something is broken and you fix it, or something keeps changing and you force it not to change. I do not think either applies to hashcpy(). Perhaps "updating", if we really wanted to say it, but because this change is not about preparing us to any planned switch of hash function, I'd suggest dropping those two lines starting from "We can benefit from...".

> Leave ppc/sha1.c as it is, because the function is about the
> SHA-1 hash algorithm whose output is and will always be 20-byte.

Correct.