RE: [PATCH] transport-helper: add trailing --

2014-05-15 Thread Felipe Contreras
Stepan Kasal wrote:
 From: Sverre Rabbelier srabbel...@gmail.com
 Date: Sat, 28 Aug 2010 20:49:01 -0500
 
 [PT: ensure we add an additional element to the argv array]
 
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
 
 Hi,
   this patch was present in msysgit from Mar 2012.
 Do you like it?

Adding an extra '--' would make sense if we do something like
'git fast-export $stuff master' and there was a file named master.
However, we do 'git fast-export $stuff refs/heads/master', which case
the '--' is a no-nop *always*.

It doesn't hurt to add it though. I think.

-- 
Felipe Contreras
--
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] transport-helper: add trailing --

2014-05-15 Thread Jeff King
On Thu, May 15, 2014 at 07:32:14AM +0200, Stepan Kasal wrote:

 From: Sverre Rabbelier srabbel...@gmail.com
 Date: Sat, 28 Aug 2010 20:49:01 -0500
 
 [PT: ensure we add an additional element to the argv array]
 
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
 
 Hi,
   this patch was present in msysgit from Mar 2012.
 Do you like it?
 I'm sorry, there is no author signoff; is the patch small enough?

It needs an explanation in the commit message, too. As Felipe noted, I
do not think it would help with ambiguity, but it should not hurt, and
is a reasonable defensive thing to do (but I did not think about it too
long, so maybe Sverre has an example that needs it).

 diff --git a/transport-helper.c b/transport-helper.c
 index 0e7c330..a01ea47 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -429,7 +429,7 @@ static int get_exporter(struct transport *transport,
   /* we need to duplicate helper-in because we want to use it after
* fastexport is done with it. */
   fastexport-out = dup(helper-in);
 - fastexport-argv = xcalloc(6 + revlist_args-nr, 
 sizeof(*fastexport-argv));
 + fastexport-argv = xcalloc(7 + revlist_args-nr, 
 sizeof(*fastexport-argv));

It would be nice if this were an argv_array so we would not have to
worry about managing the array size. This is one of several spots that
leaks array memory that I have been meaning to fix.

I just posted a series that addresses those leaks and converts this
site. I do not want to hold your patch hostage to my series, but
depending on the review on my series, you may want to re-roll this on
top; you would drop the line above, and change:

 + fastexport-argv[argc++] = --;

to:

argv_array_push(fastexport-args, --);

-Peff
--
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] transport-helper: add trailing --

2014-05-15 Thread Stepan Kasal
Hi Peff,

On Thu, May 15, 2014 at 05:00:13AM -0400, Jeff King wrote:
 I just posted a series that addresses those leaks and converts this
 site. I do not want to hold your patch hostage to my series, [...]

no problem, let's return to it later.

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