Re: [PATCH 1/5] archive-tar: use our own cmd.buf in error message

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Use the "cmd.buf" we just created in this function, instead of the
> argv[0], which we assigned "cmd.buf" for. This is in preparation for
> getting rid of the use of "argv" in this function.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  archive-tar.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index 05d2455870d..4154d9a0953 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -447,7 +447,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
>  	filter.in = -1;
>  
>  	if (start_command(&filter) < 0)
> -		die_errno(_("unable to start '%s' filter"), argv[0]);
> +		die_errno(_("unable to start '%s' filter"), cmd.buf);
>  	close(1);
>  	if (dup2(filter.in, 1) < 0)
>  		die_errno(_("unable to redirect descriptor"));
> @@ -457,7 +457,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
>  
>  	close(1);
>  	if (finish_command(&filter) != 0)
> -		die(_("'%s' filter reported error"), argv[0]);
> +		die(_("'%s' filter reported error"), cmd.buf);
>  
>  	strbuf_release(&cmd);
>  	return r;

Is this really needed to be a separate step?  If we update this
function to

 - lose local argv[] array;
 - instead use the embedded filter.args

i.e. something like the attached, such a "modern" code that uses
child_process .args would not require further changes when the
run-command API is streamlined by the remainder of the series, no?

IOW, if can arrange this step to be trivially and obviously correct
so that it can go in without fixing any bugs (if exists) in the main
part of the series, and the API update would not require such code
that already uses cp.args not cp.argv, that would be much easier to
grok.  With this "this is half a change in preparation", reviewers
need to keep this promise in their heads that argv[] will be then
removed from the function.

 archive-tar.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git c/archive-tar.c w/archive-tar.c
index 05d2455870..3c74db1746 100644
--- c/archive-tar.c
+++ w/archive-tar.c
@@ -430,7 +430,6 @@ static int write_tar_filter_archive(const struct archiver *ar,
 {
 	struct strbuf cmd = STRBUF_INIT;
 	struct child_process filter = CHILD_PROCESS_INIT;
-	const char *argv[2];
 	int r;
 
 	if (!ar->data)
@@ -440,14 +439,12 @@ static int write_tar_filter_archive(const struct archiver *ar,
 	if (args->compression_level >= 0)
 		strbuf_addf(&cmd, " -%d", args->compression_level);
 
-	argv[0] = cmd.buf;
-	argv[1] = NULL;
-	filter.argv = argv;
+	strvec_push(&filter.args, cmd.buf);
 	filter.use_shell = 1;
 	filter.in = -1;
 
 	if (start_command(&filter) < 0)
-		die_errno(_("unable to start '%s' filter"), argv[0]);
+		die_errno(_("unable to start '%s' filter"), cmd.buf);
 	close(1);
 	if (dup2(filter.in, 1) < 0)
 		die_errno(_("unable to redirect descriptor"));
@@ -457,7 +454,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
 
 	close(1);
 	if (finish_command(&filter) != 0)
-		die(_("'%s' filter reported error"), argv[0]);
+		die(_("'%s' filter reported error"), cmd.buf);
 
 	strbuf_release(&cmd);
 	return r;