Re: [PATCH 02/15] for-each-ref: don't print out elements directly

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

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> Currently, the entire callchain starting from show_ref() parses and
> prints immediately.  This inflexibility limits our ability to extend the
> parser.  So, convert the entire callchain to accept a strbuf argument to
> write to.  Also introduce a show_refs() helper that calls show_ref() in
> a loop to avoid cluttering up cmd_for_each_ref() with the task of
> initializing/freeing the strbuf.
>
> [rr: commit message]
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  builtin/for-each-ref.c | 55 ++++++++++++++++++++++++++++++++------------------
>  1 file changed, 35 insertions(+), 20 deletions(-)
>
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 1d4083c..e2d6c5a 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -864,31 +864,31 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs
>  	qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
>  }
>  
> -static void print_value(struct refinfo *ref, int atom, int quote_style)
> +static void print_value(struct strbuf *sb, struct refinfo *ref,
> +			int atom, int quote_style)
>  {
>  	struct atom_value *v;
> -	struct strbuf sb = STRBUF_INIT;
>  	get_value(ref, atom, &v);
>  	switch (quote_style) {
>  	case QUOTE_NONE:
> -		fputs(v->s, stdout);
> +		strbuf_addstr(sb, v->s);
>  		break;
>  	case QUOTE_SHELL:
> -		sq_quote_buf(&sb, v->s);
> +		sq_quote_buf(sb, v->s);
>  		break;
>  	case QUOTE_PERL:
> -		perl_quote_buf(&sb, v->s);
> +		perl_quote_buf(sb, v->s);
>  		break;
>  	case QUOTE_PYTHON:
> -		python_quote_buf(&sb, v->s);
> +		python_quote_buf(sb, v->s);
>  		break;
>  	case QUOTE_TCL:
> -		tcl_quote_buf(&sb, v->s);
> +		tcl_quote_buf(sb, v->s);
>  		break;
>  	}
>  	if (quote_style != QUOTE_NONE) {
> -		fputs(sb.buf, stdout);
> -		strbuf_release(&sb);
> +		fputs(sb->buf, stdout);
> +		strbuf_release(sb);
>  	}
>  }

The change in the first step made some sense; if asked not to quote,
it just emits the atom with fputs() in the switch (quote_style), so
there is nothing more to do after the switch.

But this change cannot be correct.  It prints literally to sb, and
leaves the function without emitting anything nor freeing the
resource in sb.