Re: [PATCH] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
Am 18.07.2014 21:10, schrieb Junio C Hamano: > René Scharfe writes: > >> Use the existing argv_array member instead of providing our own. This >> way we don't have to initialize or clean it up explicitly. >> >> Signed-off-by: Rene Scharfe >> --- > > The change below looks so trivial and I cannot offhand see why it > would break t9020 in a reproducible way. > > Puzzled... > >> remote-testsvn.c | 11 --- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/remote-testsvn.c b/remote-testsvn.c >> index 6be55cb..31415bd 100644 >> --- a/remote-testsvn.c >> +++ b/remote-testsvn.c >> @@ -175,7 +175,6 @@ static int cmd_import(const char *line) >> char *note_msg; >> unsigned char head_sha1[20]; >> unsigned int startrev; >> -struct argv_array svndump_argv = ARGV_ARRAY_INIT; >> struct child_process svndump_proc; >> >> if (read_ref(private_ref, head_sha1)) >> @@ -202,11 +201,10 @@ static int cmd_import(const char *line) >> } else { >> memset(&svndump_proc, 0, sizeof(struct child_process)); >> svndump_proc.out = -1; >> -argv_array_push(&svndump_argv, "svnrdump"); >> -argv_array_push(&svndump_argv, "dump"); >> -argv_array_push(&svndump_argv, url); >> -argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev); >> -svndump_proc.argv = svndump_argv.argv; >> +argv_array_push(&svndump_proc.args, "svnrdump"); >> +argv_array_push(&svndump_proc.args, "dump"); >> +argv_array_push(&svndump_proc.args, url); >> +argv_array_pushf(&svndump_proc.args, "-r%u:HEAD", startrev); >> >> code = start_command(&svndump_proc); >> if (code) >> @@ -227,7 +225,6 @@ static int cmd_import(const char *line) >> code = finish_command(&svndump_proc); >> if (code) >> warning("%s, returned %d", svndump_proc.argv[0], code); >> -argv_array_clear(&svndump_argv); Unfortunately I don't get a test failure, but I think I see what's wrong: The warning line above references the argv array after it was freed by finish_command. Ouch. Fixup below: --- remote-testsvn.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/remote-testsvn.c b/remote-testsvn.c index 31415bd..e3ad11b 100644 --- a/remote-testsvn.c +++ b/remote-testsvn.c @@ -176,6 +176,7 @@ static int cmd_import(const char *line) unsigned char head_sha1[20]; unsigned int startrev; struct child_process svndump_proc; + const char *command; if (read_ref(private_ref, head_sha1)) startrev = 0; @@ -199,16 +200,17 @@ static int cmd_import(const char *line) if(dumpin_fd < 0) die_errno("Couldn't open svn dump file %s.", url); } else { + command = "svnrdump"; memset(&svndump_proc, 0, sizeof(struct child_process)); svndump_proc.out = -1; - argv_array_push(&svndump_proc.args, "svnrdump"); + argv_array_push(&svndump_proc.args, command); argv_array_push(&svndump_proc.args, "dump"); argv_array_push(&svndump_proc.args, url); argv_array_pushf(&svndump_proc.args, "-r%u:HEAD", startrev); code = start_command(&svndump_proc); if (code) - die("Unable to start %s, code %d", svndump_proc.argv[0], code); + die("Unable to start %s, code %d", command, code); dumpin_fd = svndump_proc.out; } /* setup marks file import/export */ @@ -224,7 +226,7 @@ static int cmd_import(const char *line) if (!dump_from_file) { code = finish_command(&svndump_proc); if (code) - warning("%s, returned %d", svndump_proc.argv[0], code); + warning("%s, returned %d", command, code); } return 0; -- 2.0.2 -- 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] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
René Scharfe writes: > Use the existing argv_array member instead of providing our own. This > way we don't have to initialize or clean it up explicitly. > > Signed-off-by: Rene Scharfe > --- The change below looks so trivial and I cannot offhand see why it would break t9020 in a reproducible way. Puzzled... > remote-testsvn.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/remote-testsvn.c b/remote-testsvn.c > index 6be55cb..31415bd 100644 > --- a/remote-testsvn.c > +++ b/remote-testsvn.c > @@ -175,7 +175,6 @@ static int cmd_import(const char *line) > char *note_msg; > unsigned char head_sha1[20]; > unsigned int startrev; > - struct argv_array svndump_argv = ARGV_ARRAY_INIT; > struct child_process svndump_proc; > > if (read_ref(private_ref, head_sha1)) > @@ -202,11 +201,10 @@ static int cmd_import(const char *line) > } else { > memset(&svndump_proc, 0, sizeof(struct child_process)); > svndump_proc.out = -1; > - argv_array_push(&svndump_argv, "svnrdump"); > - argv_array_push(&svndump_argv, "dump"); > - argv_array_push(&svndump_argv, url); > - argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev); > - svndump_proc.argv = svndump_argv.argv; > + argv_array_push(&svndump_proc.args, "svnrdump"); > + argv_array_push(&svndump_proc.args, "dump"); > + argv_array_push(&svndump_proc.args, url); > + argv_array_pushf(&svndump_proc.args, "-r%u:HEAD", startrev); > > code = start_command(&svndump_proc); > if (code) > @@ -227,7 +225,6 @@ static int cmd_import(const char *line) > code = finish_command(&svndump_proc); > if (code) > warning("%s, returned %d", svndump_proc.argv[0], code); > - argv_array_clear(&svndump_argv); > } > > return 0; -- 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] remote-testsvn: use internal argv_array of struct child_process in cmd_import()
Use the existing argv_array member instead of providing our own. This way we don't have to initialize or clean it up explicitly. Signed-off-by: Rene Scharfe --- remote-testsvn.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/remote-testsvn.c b/remote-testsvn.c index 6be55cb..31415bd 100644 --- a/remote-testsvn.c +++ b/remote-testsvn.c @@ -175,7 +175,6 @@ static int cmd_import(const char *line) char *note_msg; unsigned char head_sha1[20]; unsigned int startrev; - struct argv_array svndump_argv = ARGV_ARRAY_INIT; struct child_process svndump_proc; if (read_ref(private_ref, head_sha1)) @@ -202,11 +201,10 @@ static int cmd_import(const char *line) } else { memset(&svndump_proc, 0, sizeof(struct child_process)); svndump_proc.out = -1; - argv_array_push(&svndump_argv, "svnrdump"); - argv_array_push(&svndump_argv, "dump"); - argv_array_push(&svndump_argv, url); - argv_array_pushf(&svndump_argv, "-r%u:HEAD", startrev); - svndump_proc.argv = svndump_argv.argv; + argv_array_push(&svndump_proc.args, "svnrdump"); + argv_array_push(&svndump_proc.args, "dump"); + argv_array_push(&svndump_proc.args, url); + argv_array_pushf(&svndump_proc.args, "-r%u:HEAD", startrev); code = start_command(&svndump_proc); if (code) @@ -227,7 +225,6 @@ static int cmd_import(const char *line) code = finish_command(&svndump_proc); if (code) warning("%s, returned %d", svndump_proc.argv[0], code); - argv_array_clear(&svndump_argv); } return 0; -- 2.0.0 -- 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