Re: [PATCH v3] mailinfo: fix pointential memory leak if `decode_header` failed

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

"Lidong Yan via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -static struct strbuf *decode_q_segment(const struct strbuf *q_seg, int rfc2047)
> +static int decode_q_segment(struct strbuf *out, const struct strbuf *q_seg,
> +			    int rfc2047)
>  {
>  	const char *in = q_seg->buf;
>  	int c;
> -	struct strbuf *out = xmalloc(sizeof(struct strbuf));
>  	strbuf_init(out, q_seg->len);

This strbuf_init() is still here?

> +static int decode_b_segment(struct strbuf *out, const struct strbuf *b_seg)
>  {
>  	/* Decode in..ep, possibly in-place to ot */
>  	int c, pos = 0, acc = 0;
>  	const char *in = b_seg->buf;
> -	struct strbuf *out = xmalloc(sizeof(struct strbuf));
>  	strbuf_init(out, b_seg->len);

Ditto.

> @@ -475,7 +474,7 @@ static int convert_to_utf8(struct mailinfo *mi,
>  static void decode_header(struct mailinfo *mi, struct strbuf *it)
>  {
>  	char *in, *ep, *cp;
> -	struct strbuf outbuf = STRBUF_INIT, *dec;
> +	struct strbuf outbuf = STRBUF_INIT, dec = STRBUF_INIT;

Initializing this at the caller side is good.

> @@ -530,18 +529,20 @@ static void decode_header(struct mailinfo *mi, struct strbuf *it)
>  		default:
>  			goto release_return;
>  		case 'b':
> -			dec = decode_b_segment(&piecebuf);
> +			if ((found_error = decode_b_segment(&dec, &piecebuf)))
> +				goto release_return;
>  			break;
>  		case 'q':
> -			dec = decode_q_segment(&piecebuf, 1);
> +			if ((found_error = decode_q_segment(&dec, &piecebuf, 1)))
> +				goto release_return;
>  			break;
>  		}
> -		if (convert_to_utf8(mi, dec, charset_q.buf))
> +		if (convert_to_utf8(mi, &dec, charset_q.buf)) {
>  			goto release_return;
> +		}

Don't enclose a single statement block inside {braces}.


Thanks.