Re: [PATCH v2 7/7] format-patch: use stdout directly

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 20, 2016, 18:57 UTC
Message-ID: <xmqqshw7u1s2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <d486373d9eb2bd248a80b6be7061b56db2f148aa.1466420060.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 11 quoted lines

> Earlier, we freopen()ed stdout in order to write patches to files.
> That forced us to duplicate stdout (naming it "realstdout") because we
> *still* wanted to be able to report the file names.
>
> As we do not abuse stdout that way anymore, we no longer need to
> duplicate stdout, either.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/log.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

Overall, it was a pleasant read, modulo some details that I mentioned separately elsewhere, i.e.

 - we do not need to lose flush-or-die
 - we do not need to lose --color=always
 - we do not need to lose macro-ness of putchar()

Thanks.

Show 32 quoted lines

>
> diff --git a/builtin/log.c b/builtin/log.c
> index db034a8..5a889d5 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -796,7 +796,6 @@ static int git_format_config(const char *var, const char *value, void *cb)
>  	return git_log_config(var, value, cb);
>  }
>  
> -static FILE *realstdout = NULL;
>  static const char *output_directory = NULL;
>  static int outdir_offset;
>  
> @@ -822,7 +821,7 @@ static int open_next_file(struct commit *commit, const char *subject,
>  		fmt_output_subject(&filename, subject, rev);
>  
>  	if (!quiet)
> -		fprintf(realstdout, "%s\n", filename.buf + outdir_offset);
> +		printf("%s\n", filename.buf + outdir_offset);
>  
>  	if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL)
>  		return error(_("Cannot open patch file %s"), filename.buf);
> @@ -1629,9 +1628,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  		get_patch_ids(&rev, &ids);
>  	}
>  
> -	if (!use_stdout)
> -		realstdout = xfdopen(xdup(1), "w");
> -
>  	if (prepare_revision_walk(&rev))
>  		die(_("revision walk setup failed"));
>  	rev.boundary = 1;