Re: [PATCH 6/6] strbuf-safe: add init and release methods

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 21, 2026, 21:44 UTC
Message-ID: <xmqqld8ul1ny.fsf@gitster.g>
In-Reply-To: <dea925f31647e7c08f3fa467b8058351b463f593.1789736540.git.gitgitgadget@gmail.com>

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

> +int jw_release(struct json_writer *jw)
>  {
> -	strbuf_release(&jw->json);
> -	strbuf_release(&jw->open_stack);
> +	enum safe_result result = SUCCESS;
> +
> +	/* attempt both removals without short-circuiting. */
> +	result = sstrbuf_release(&jw->json) || result;
> +	result = sstrbuf_release(&jw->open_stack) || result;
> +
> +	return result;
>  }

This is puzzling in a few ways.

"enum safe_result" so far has been SUCCESS==0 and MEMORY_ERROR==1. Presumably in some future we would gain other kind of error symbols, but when that happens is this meant to act as an enumeration of different kinds errors? Or an enumeration of bitmasks that can signal different kinds of errors?

If we mean "enum safe_result" is an enumeration of different kinds of errors, then the "result" variable and the returned value from here would be able to report a *single* kind of error, and it may be common to report the first error we encounter, in which case

    enum safe_result result = SUCCESS;
    enum safe_result res;

    res = sstrbuf_release(&jw->json);
    if (!result && res)
	result = res;
    res = sstrbuf_release(&jw->open_stack);
    if (!result && res)
	result = res;
    return result;

would be slightly longer, far easier to reason about, and is a lot more futureproof. What you wrote, with "||", does not really allow anything other than "is it still zero, or coalesce any non-zero value to 1".

On the other hand, if we mean "enum safe_result" is an enumeration of bitmasks, each bit representing different kind of error, then

    enum safe_result result = 0;

    result |= sstrbuf_release(&jw->json);
    result |= sstrbuf_release(&jw->open_stack);
    return result;

would probably be what you want. That way you can add different functions that returns different bit to signal a different kind of error and or it in.

    result |= some_function();