[PATCH] Replace memcpy with hashcpy when lengths defined

2014-03-02 Thread Alberto 
From: Alberto Corona albco...@gmail.com

Replaced memcpy with hashcpy where lengts in memcpy
are already defined.

Signed-off-by: Alberto Corona albco...@gmail.com
---
 bundle.c| 2 +-
 grep.c  | 2 +-
 refs.c  | 2 +-
 sha1_name.c | 4 ++--
 4 files changed, 5 insertions(+), 5 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/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;
 }
 
diff --git a/sha1_name.c b/sha1_name.c
index 6fca869..3f5010f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -111,7 +111,7 @@ static void find_short_object_filename(int len, const char 
*hex_pfx, struct disa
continue;
if (memcmp(de-d_name, hex_pfx + 2, len - 2))
continue;
-   memcpy(hex + 2, de-d_name, 38);
+   hashcpy(hex + 2, de-d_name);
if (!get_sha1_hex(hex, sha1))
update_candidates(ds, sha1);
}
@@ -373,7 +373,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, 
int len)
static char hex[41];
 
exists = has_sha1_file(sha1);
-   memcpy(hex, sha1_to_hex(sha1), 40);
+   hashcpy(hex, sha1_to_hex(sha1));
if (len == 40 || !len)
return hex;
while (len  40) {
-- 
1.9.0.138.g2de3478.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Replace memcpy with hashcpy when lengths defined

2014-03-02 Thread Philip Oakley 

From: Alberto albco...@gmail.com

From: Alberto Corona albco...@gmail.com

Replaced memcpy with hashcpy where lengts in memcpy


s/lengts/lengths/


are already defined.

Signed-off-by: Alberto Corona albco...@gmail.com
---
bundle.c| 2 +-
grep.c  | 2 +-
refs.c  | 2 +-
sha1_name.c | 4 ++--
4 files changed, 5 insertions(+), 5 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/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;
}

diff --git a/sha1_name.c b/sha1_name.c
index 6fca869..3f5010f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -111,7 +111,7 @@ static void find_short_object_filename(int len, 
const char *hex_pfx, struct disa

 continue;
 if (memcmp(de-d_name, hex_pfx + 2, len - 2))
 continue;
- memcpy(hex + 2, de-d_name, 38);
+ hashcpy(hex + 2, de-d_name);
 if (!get_sha1_hex(hex, sha1))
 update_candidates(ds, sha1);
 }
@@ -373,7 +373,7 @@ const char *find_unique_abbrev(const unsigned char 
*sha1, int len)

 static char hex[41];

 exists = has_sha1_file(sha1);
- memcpy(hex, sha1_to_hex(sha1), 40);
+ hashcpy(hex, sha1_to_hex(sha1));
 if (len == 40 || !len)
 return hex;
 while (len  40) {
--
1.9.0.138.g2de3478.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Replace memcpy with hashcpy when lengths defined

2014-03-02 Thread Eric Sunshine 
On Sun, Mar 2, 2014 at 2:19 PM, Alberto albco...@gmail.com wrote:
 From: Alberto Corona albco...@gmail.com

 Replaced memcpy with hashcpy where lengts in memcpy
 are already defined.

This doesn't really explain what this patch is attempting to do. What
does lengths already defined mean? It's also misleading if taken
literally (as seen below).

Instead, you should say something about the purpose of the change. For
instance, explain that the change takes advantage of the abstraction
provided by hashcpy() rather than hardcoding knowledge about a
particular hash representation.

More below.

 Signed-off-by: Alberto Corona albco...@gmail.com
 ---
  bundle.c| 2 +-
  grep.c  | 2 +-
  refs.c  | 2 +-
  sha1_name.c | 4 ++--
  4 files changed, 5 insertions(+), 5 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/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;
  }

 diff --git a/sha1_name.c b/sha1_name.c
 index 6fca869..3f5010f 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -111,7 +111,7 @@ static void find_short_object_filename(int len, const 
 char *hex_pfx, struct disa
 continue;
 if (memcmp(de-d_name, hex_pfx + 2, len - 2))
 continue;
 -   memcpy(hex + 2, de-d_name, 38);
 +   hashcpy(hex + 2, de-d_name);

This can't be correct. hashcpy() copies the binary representation of a
hash (which is currently 20 bytes, as seen in the implementation of
hashcpy() in cache.h). The fact that this particular memcpy() is
copying 38 bytes should be a clue that something is different here. In
fact, for this case, 'hex' is a 40-character textual representation of
the hash, thus not suitable for hashcpy().

 if (!get_sha1_hex(hex, sha1))
 update_candidates(ds, sha1);
 }
 @@ -373,7 +373,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, 
 int len)
 static char hex[41];

 exists = has_sha1_file(sha1);
 -   memcpy(hex, sha1_to_hex(sha1), 40);
 +   hashcpy(hex, sha1_to_hex(sha1));

Same as above.

 if (len == 40 || !len)
 return hex;
 while (len  40) {
 --
 1.9.0.138.g2de3478.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html