threads / patch / 36661

patchtransport-helper: add trailing --

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

## tl;dr

6 messages between May 15, 2014 and May 16, 2014. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Stepan Kasal· May 15, 2014, 05:32 UTC · lore
From: Sverre Rabbelier <srabbelier@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 <kasal@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?
Stepan
 transport-helper.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Show changes to transport-helper.c +3 −1
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));
 	fastexport->argv[argc++] = "fast-export";
 	fastexport->argv[argc++] = "--use-done-feature";
 	fastexport->argv[argc++] = data->signed_tags ?
@@ -446,6 +446,8 @@ static int get_exporter(struct transport *transport,
 	for (i = 0; i < revlist_args->nr; i++)
 		fastexport->argv[argc++] = revlist_args->items[i].string;
 
+	fastexport->argv[argc++] = "--";
+
 	fastexport->git_cmd = 1;
 	return start_command(fastexport);
 }
-- 
1.9.2.msysgit.0.335.gd2a461f

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Felipe Contreras· May 15, 2014, 06:05 UTC · re: Stepan Kasal · lore

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

Stepan Kasal wrote:
Show 11 quoted lines
> From: Sverre Rabbelier <srabbelier@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 <kasal@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

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Jeff King· May 15, 2014, 09:00 UTC · re: Stepan Kasal · lore

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

On Thu, May 15, 2014 at 07:32:14AM +0200, Stepan Kasal wrote:
Show 12 quoted lines
> From: Sverre Rabbelier <srabbelier@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 <kasal@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).

Show 10 quoted lines
> 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
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Stepan Kasal· May 15, 2014, 09:16 UTC · re: Jeff King · lore

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

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
-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Johannes Schindelin· May 15, 2014, 18:28 UTC · re: Stepan Kasal · lore

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

Hi,
On Thu, 15 May 2014, Stepan Kasal wrote:
> From: Sverre Rabbelier <srabbelier@gmail.com>
> Date: Sat, 28 Aug 2010 20:49:01 -0500
> 
> [PT: ensure we add an additional element to the argv array]
IIRC we had problems with refs vs file names.

Ciao, Johannes

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Felipe Contreras· May 16, 2014, 13:15 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin wrote:
Show 10 quoted lines
> Hi,
> 
> On Thu, 15 May 2014, Stepan Kasal wrote:
> 
> > From: Sverre Rabbelier <srabbelier@gmail.com>
> > Date: Sat, 28 Aug 2010 20:49:01 -0500
> > 
> > [PT: ensure we add an additional element to the argv array]
> 
> IIRC we had problems with refs vs file names.

Actually I tried to push a file named refs/heads/master and I saw the issue. I'd say it's a very rare issue, and I don't see how it could be triggered with normal files since all the ref names have the full name. That being said I don't think it would hurt.

-- 
Felipe Contreras

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

← back to recent threads