From: Junio C Hamano <gitster@pobox•com>
To: Sun He <sunheehnus@gmail•com>
Cc: git@vger•kernel.org, sunshine@sunshineco•com,
mhagger@alum•mit.edu, pclouds@gmail•com
Subject: Re: [PATCH v3] Replace memcpy with hashcpy when dealing hash copy globally
Date: Wed, 05 Mar 2014 13:48:29 -0800 [thread overview]
Message-ID: <xmqqfvmwfhaq.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1393839599-6955-1-git-send-email-sunheehnus@gmail.com> (Sun He's message of "Mon, 3 Mar 2014 17:39:59 +0800")
Sun He <sunheehnus@gmail•com> writes:
> Replacing memcpy with hashcpy is more directly and elegant.
Can we justify the change without being subjective?
> Leave ppc/sha1.c alone, as it is an isolated component.
> Pull cache.h(actually ../cache.h) in just for one memcpy
> there is not proper.
That is not the reason why that memcpy of 20-byte must stay as it
is. If for whatever reason we later choose to switch to using
another hash function, say MD5, to come up with the object names,
the majority of memcpy(..., 20) must change to copy 16 bytes, and it
makes sense to contain that implementation-specific knowledge of
twenty behind the hashcpy() abstraction. The 20-byte memcpy() call
in ppc/sha1.c, however, is an implementation of *THE* SHA-1
algorithm, whose output is and will always be 20-byte. It will not
change when we decide to replace what hash function is used to name
our objects (which would result in updating the implementation of
hashcpy()). That is the reason why you shouldn't touch that one.
It has to be explicitly 20 byte, without ever getting affected by
what length our hashcpy() may choose to copy.
Perhaps...
We invented hashcpy() to keep the abstraction of "object
name" behind it. Use it instead of calling memcpy() with
hard-coded 20-byte length when moving object names between
pieces of memory.
Leave ppc/sha1.c as-is, because the function *is* about
*the* SHA-1 hash algorithm whose output is and will always
be 20-byte.
or something.
> Find the potential places with memcpy by the bash command:
> $ find . | xargs grep "memcpy.*\(.*20.*\)"
If you are planning to hack on git, learn how to use it first ;-)
$ git grep 'memcpy.*, 20)'
prev parent reply other threads:[~2014-03-05 21:48 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-03 9:39 [PATCH v3] Replace memcpy with hashcpy when dealing hash copy globally Sun He
2014-03-05 21:48 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfvmwfhaq.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=mhagger@alum$(echo .)mit.edu \
--cc=pclouds@gmail$(echo .)com \
--cc=sunheehnus@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox