Re: [PATCH v2 24/25] register_ref(): make a copy of the bad reference SHA-1

2013-05-30 Thread Michael Haggerty 
On 05/29/2013 06:53 PM, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 [...]
 +current_bad_sha1 = xmalloc(20);
 +hashcpy(current_bad_sha1, sha1);
 
 This, together with 18/25, may hint that we want hashdup()?

I thought about it, but was surprised that I could only find one or two
other places in the existing code that would use such a function.  But
sure, I would be happy to submit a patch.

I think hashdup() needn't be inline, so the definition can't go with its
cousins in cache.h.  There is no cache.c.  So where would be the best
place to define hashdup()?  object.c?  sha1_name.c?

While I'm at it, I think it would be nice to have constants like

#define SHA1_LEN 20
#define SHA1_HEX_LEN 40

and start using those instead of magic numbers.  Any objections (or
suggestions for better names)?

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 v2 24/25] register_ref(): make a copy of the bad reference SHA-1

2013-05-30 Thread Philip Oakley 

From: Michael Haggerty mhag...@alum.mit.edu
Sent: Thursday, May 30, 2013 10:51 PM

On 05/29/2013 06:53 PM, Junio C Hamano wrote:

Michael Haggerty mhag...@alum.mit.edu writes:

[...]
+ current_bad_sha1 = xmalloc(20);
+ hashcpy(current_bad_sha1, sha1);


This, together with 18/25, may hint that we want hashdup()?


I thought about it, but was surprised that I could only find one or 
two

other places in the existing code that would use such a function.  But
sure, I would be happy to submit a patch.

I think hashdup() needn't be inline, so the definition can't go with 
its

cousins in cache.h.  There is no cache.c.  So where would be the best
place to define hashdup()?  object.c?  sha1_name.c?

While I'm at it, I think it would be nice to have constants like

#define SHA1_LEN 20
#define SHA1_HEX_LEN 40

and start using those instead of magic numbers.  Any objections (or
suggestions for better names)?



The first named constant should be fully qualified to the same extent as 
the second, perhaps:

   #define SHA1_BYTE_LEN 20

and perhaps with an alternate of (though HEX is just as good):
   #define SHA1_CHAR_LEN 40



Michael

--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
Philip 


--
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

[PATCH v2 24/25] register_ref(): make a copy of the bad reference SHA-1

2013-05-25 Thread Michael Haggerty 
The lifetime of the sha1 parameter passed to an each_ref_fn callback
is not guaranteed, so make a copy for later use.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 bisect.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/bisect.c b/bisect.c
index 374d9e2..71c1958 100644
--- a/bisect.c
+++ b/bisect.c
@@ -15,7 +15,7 @@
 static struct sha1_array good_revs;
 static struct sha1_array skipped_revs;
 
-static const unsigned char *current_bad_sha1;
+static unsigned char *current_bad_sha1;
 
 static const char *argv_checkout[] = {checkout, -q, NULL, --, NULL};
 static const char *argv_show_branch[] = {show-branch, NULL, NULL};
@@ -404,7 +404,8 @@ static int register_ref(const char *refname, const unsigned 
char *sha1,
int flags, void *cb_data)
 {
if (!strcmp(refname, bad)) {
-   current_bad_sha1 = sha1;
+   current_bad_sha1 = xmalloc(20);
+   hashcpy(current_bad_sha1, sha1);
} else if (!prefixcmp(refname, good-)) {
sha1_array_append(good_revs, sha1);
} else if (!prefixcmp(refname, skip-)) {
-- 
1.8.2.3

--
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