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

2014-03-01 10:58 GMT+08:00 Duy Nguyen [off-list ref]:

On Sat, Mar 1, 2014 at 8:07 AM, Sun He [off-list ref] wrote:

quoted

Signed-off-by: Sun He <redacted>
---
 Find the potential places with memcpy by the bash command:
   $ find . | xargs grep "memcpy.*\(.*20.*\)"

 Helped-by: Michael Haggerty[off-list ref]

You may want to put this Helped-by before "---" because it's supposed
to end up in the final commit. The patch looks straightforward,
except..

Yeah, got it.
Thanks.

quoted

diff --git a/ppc/sha1.c b/ppc/sha1.c
index ec6a192..8a87fea 100644
--- a/ppc/sha1.c
+++ b/ppc/sha1.c

@@ -9,6 +9,7 @@
 #include <stdio.h>
 #include <string.h>
 #include "sha1.h"
+#include "cache.h"

 extern void ppc_sha1_core(uint32_t *hash, const unsigned char *p,
                          unsigned int nblocks);

@@ -67,6 +68,6 @@ int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c)
                memset(&c->buf.b[cnt], 0, 56 - cnt);
        c->buf.l[7] = c->len;
        ppc_sha1_core(c->hash, c->buf.b, 1);
-       memcpy(hash, c->hash, 20);
+       hashcpy(hash, c->hash);
        return 0;
 }

cache.h (actually git-compat-util.h that cache.h includes) messes
around with system headers by defining this and that macro. The
general rule is if cache.h or git-compat-util.h is included, it's the
first #include, and system includes will be always in
git-compat-util.h (grep '^#include' shows this). Maybe it's best to
leave this memcpy alone (and if you do, state so in the commit message
with the reason).

Yap, I should follow the general rule. My fault.
Thanks.

What's more,

I have find out all the files that cache.h git-compat-util.h and
builtin.h are not the first #include

==> test-sigchain.c <==
#include "sigchain.h"
==> sigchain.c <==
#include "sigchain.h"

And I checked "sigchain.h", that it includes very little information.
It didn't import any potential errors. But I think it should be placed
after "cache.h" to match the consistence of the general rule.

--
Duy