git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Mar 3, 2014, 07:49 UTC
Message-ID
<CAPig+cTiC9-wmeXANFMZO-1MtLQ+u40aQi1jr0JPfK4KS+DS-w@mail.gmail.com>
In-Reply-To
<1393753044-6232-1-git-send-email-sunheehnus@gmail.com>
On Sun, Mar 2, 2014 at 4:37 AM, Sun He <sunheehnus@gmail.com> wrote:
>  Replacing memcpy with hashcpy is more directly and elegant.

A better explanation is that the change takes advantage of the abstraction provided by hashcpy() rather than hardcoding knowledge about a particular hash representation.

>  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.
Strange one-space indentation on all lines of commit message.
Other than that, the patch looks reasonable.
Show 107 quoted lines
> Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
> Helped-by: Duy Nguyen <pclouds@gmail.com>
> Signed-off-by: Sun He <sunheehnus@gmail.com>
> ---
>
>  PATCH v2 leave ppc/sha1.c alone.
>
>  The general rule is if cache.h or git-compat-util.h is included,
>  it is the first #include, and system includes will be always in
>  git-compat-tuil.h.
>                                         via Duy Nguyen
>
>  The change in PATCH v1 is not proper because I placed cache.h
>  in the end.
>  And adding it to the head is not a good way to achieve the goal,
>  as is said above "---".
>
>  Thanks to Duy Nguyen.
>
>  Find the potential places with memcpy by the bash command:
>   $ find . | xargs grep "memcpy.*\(.*20.*\)"
>
>
>  ppc/sha1.c doesn't include cache.h and it cannot use
>  So just leave memcpy(in ppc/sha1.c) alone
>
>  bundle.c            | 2 +-
>  grep.c              | 2 +-
>  pack-bitmap-write.c | 2 +-
>  reflog-walk.c       | 4 ++--
>  refs.c              | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index e99065c..7809fbb 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,
>                 list->list = xrealloc(list->list,
>                                 list->alloc * sizeof(list->list[0]));
>         }
> -       memcpy(list->list[list->nr].sha1, sha1, 20);
> +       hashcpy(list->list[list->nr].sha1, sha1);
>         list->list[list->nr].name = xstrdup(name);
>         list->nr++;
>  }
> diff --git a/grep.c b/grep.c
> index c668034..f5101f7 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,
>                 break;
>         case GREP_SOURCE_SHA1:
>                 gs->identifier = xmalloc(20);
> -               memcpy(gs->identifier, identifier, 20);
> +               hashcpy(gs->identifier, identifier);
>                 break;
>         case GREP_SOURCE_BUF:
>                 gs->identifier = NULL;
> diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> index 1218bef..5f1791a 100644
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
>         header.version = htons(default_version);
>         header.options = htons(flags | options);
>         header.entry_count = htonl(writer.selected_nr);
> -       memcpy(header.checksum, writer.pack_checksum, 20);
> +       hashcpy(header.checksum, writer.pack_checksum);
>
>         sha1write(f, &header, sizeof(header));
>         dump_bitmap(f, writer.commits);
> diff --git a/reflog-walk.c b/reflog-walk.c
> index b2fbdb2..d490f7d 100644
> --- a/reflog-walk.c
> +++ b/reflog-walk.c
> @@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
>                         sizeof(struct reflog_info));
>         }
>         item = array->items + array->nr;
> -       memcpy(item->osha1, osha1, 20);
> -       memcpy(item->nsha1, nsha1, 20);
> +       hashcpy(item->osha1, osha1);
> +       hashcpy(item->nsha1, nsha1);
>         item->email = xstrdup(email);
>         item->timestamp = timestamp;
>         item->tz = tz;
> diff --git a/refs.c b/refs.c
> index 89228e2..f90b7ea 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs,
>         if (ref == NULL)
>                 return -1;
>
> -       memcpy(sha1, ref->u.value.sha1, 20);
> +       hashcpy(sha1, ref->u.value.sha1);
>         return 0;
>  }
>
> --
> 1.9.0.138.g2de3478.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous: Sun He
Message 2 of 2 in “Replace memcpy with hashcpy when dealing hash copy globally”
  1. Replace memcpy with hashcpy when dealing hash copy globallySun He, Mar 2, 2014
  2. Eric SunshineMar 3, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.