{"thread":{"id":"36073","subject":"[PATCH v4] Replace memcpy with hashcpy when dealing hash copy globally","startedAt":"2014-03-06T09:11:24Z","lastAt":"2014-03-06T19:32:47Z","messageCount":2,"participants":["Sun He","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"236153","messageId":"1394097084-3129-1-git-send-email-sunheehnus@gmail.com","threadId":"36073","inReplyTo":null,"subject":"[PATCH v4] Replace memcpy with hashcpy when dealing hash copy globally","fromName":"Sun He","fromEmail":"sunheehnus@gmail.com","sentAt":"2014-03-06T09:11:24Z","receivedAt":"2014-03-06T09:11:24Z","isPatch":true,"sender":{"key":"sunheehnus@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2889804?v=4"},"body":"hashcpy() can keep the abstraction of \"object name\" behind it.\nUse it instead of memcpy() with hard-coded 20-byte length when\nmoving object names between pieces of memory.\nWe can benefit from it, when we switch to another hash algorithm,\neg. MD5, by just fixing the hashcpy().\n\nLeave ppc/sha1.c as it is, because the function is about the\nSHA-1 hash algorithm whose output is and will always be 20-byte.\n\nHelped-by: Michael Haggerty \nHelped-by: Duy Nguyen \nHelped-by: Junio C Hamano \nSigned-off-by: Sun He \n---\nPATCH v4 changed the reason why should take this patch as tutored by\nJunio C Hamano.\n Thanks to Junio C Hamano\n\nPATCH v3 delete the one-space indentation on each line of commit message\nas is suggested by Eric Sunshine.\n Thanks to Eric Sunshine.\n\nPATCH v2 leave ppc/sha1.c alone.\n\nThe general rule is if cache.h or git-compat-util.h is included,\nit is the first #include, and system includes will be always in\ngit-compat-tuil.h.\n \t\t\t\tvia Duy Nguyen\n\nThe change in PATCH v1 is not proper because I placed cache.h\nin the end.\nAnd adding it to the head is not a good way to achieve the goal,\nas is said above \"---\".\n Thanks to Duy Nguyen.\n\nFind the potential places with memcpy by the bash command:\n $ git grep 'memcpy.*20'\n Thanks to Junio C Hamano\n\n bundle.c | 2 +-\n grep.c | 2 +-\n pack-bitmap-write.c | 2 +-\n reflog-walk.c | 4 ++--\n refs.c | 2 +-\n 5 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex e99065c..7809fbb 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,\n \t\tlist->list = xrealloc(list->list,\n \t\t\t\tlist->alloc * sizeof(list->list[0]));\n \t}\n-\tmemcpy(list->list[list->nr].sha1, sha1, 20);\n+\thashcpy(list->list[list->nr].sha1, sha1);\n \tlist->list[list->nr].name = xstrdup(name);\n \tlist->nr++;\n }\ndiff --git a/grep.c b/grep.c\nindex c668034..f5101f7 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,\n \t\tbreak;\n \tcase GREP_SOURCE_SHA1:\n \t\tgs->identifier = xmalloc(20);\n-\t\tmemcpy(gs->identifier, identifier, 20);\n+\t\thashcpy(gs->identifier, identifier);\n \t\tbreak;\n \tcase GREP_SOURCE_BUF:\n \t\tgs->identifier = NULL;\ndiff --git a/pack-bitmap-write.c b/pack-bitmap-write.c\nindex 1218bef..5f1791a 100644\n--- a/pack-bitmap-write.c\n+++ b/pack-bitmap-write.c\n@@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index,\n \theader.version = htons(default_version);\n \theader.options = htons(flags | options);\n \theader.entry_count = htonl(writer.selected_nr);\n-\tmemcpy(header.checksum, writer.pack_checksum, 20);\n+\thashcpy(header.checksum, writer.pack_checksum);\n \n \tsha1write(f, &header, sizeof(header));\n \tdump_bitmap(f, writer.commits);\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex b2fbdb2..d490f7d 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,\n \t\t\tsizeof(struct reflog_info));\n \t}\n \titem = array->items + array->nr;\n-\tmemcpy(item->osha1, osha1, 20);\n-\tmemcpy(item->nsha1, nsha1, 20);\n+\thashcpy(item->osha1, osha1);\n+\thashcpy(item->nsha1, nsha1);\n \titem->email = xstrdup(email);\n \titem->timestamp = timestamp;\n \titem->tz = tz;\ndiff --git a/refs.c b/refs.c\nindex 89228e2..f90b7ea 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs,\n \tif (ref == NULL)\n \t\treturn -1;\n \n-\tmemcpy(sha1, ref->u.value.sha1, 20);\n+\thashcpy(sha1, ref->u.value.sha1);\n \treturn 0;\n }\n \n-- \n1.9.0.138.g2de3478.dirty\n"},{"id":"236185","messageId":"xmqqpplzxgv4.fsf@gitster.dls.corp.google.com","threadId":"36073","inReplyTo":"1394097084-3129-1-git-send-email-sunheehnus@gmail.com","subject":"Re: [PATCH v4] Replace memcpy with hashcpy when dealing hash copy globally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-06T19:32:47Z","receivedAt":"2014-03-06T19:32:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sun He writes:\n\n> hashcpy() can keep the abstraction of \"object name\" behind it.\n\nDo we really want to change the phrasing you took the above from to\nsay \"*can* keep\"?\n\nProviding the \"object name\" abstraction is the whole point of the\nfunction, so of course it can keep it, but that goes without\nsaying---it was the sole reason why it was invented in the first\nplace.\n\n> Use it instead of memcpy() with hard-coded 20-byte length when\n> moving object names between pieces of memory.\n> We can benefit from it, when we switch to another hash algorithm,\n> eg. MD5, by just fixing the hashcpy().\n\n\"fix\" can be used in two scenarios, I think. Something is broken\nand you fix it, or something keeps changing and you force it not to\nchange. I do not think either applies to hashcpy(). Perhaps\n\"updating\", if we really wanted to say it, but because this change\nis not about preparing us to any planned switch of hash function,\nI'd suggest dropping those two lines starting from \"We can benefit\nfrom...\".\n\n> Leave ppc/sha1.c as it is, because the function is about the\n> SHA-1 hash algorithm whose output is and will always be 20-byte.\n\nCorrect.\n"}]}