Re: [PATCH] builtin/help.c: move strbuf out of help loops
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Mar 10, 2026, 12:41 UTC
- Message-ID
- <abARj_VI9n2nB_xT@pks.im>
- In-Reply-To
- <20260310070328.29836-1-r.siddharth.shrimali@gmail.com>
On Tue, Mar 10, 2026 at 12:33:28PM +0530, Siddharth Shrimali wrote:
Show 34 quoted lines
> In list_config_help(), a strbuf was being initialized and released
> inside two separate loops. This caused unnecessary memory allocation
> and deallocation on every iteration.
>
> Move the strbuf declaration to the top of the function and use
> strbuf_reset() inside the loops to reuse the same buffer. Similarly
> release() the buffer at the end of the function to free the memory.
> This improves performance by avoiding repeated heap pressure by reducing
> the number of allocations.
>
> This also fixes a minor memory leak when the SHOW_CONFIG_HUMAN case
> triggers a continue.
>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
> builtin/help.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 86a3d03a9b..07398b430e 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -134,10 +134,10 @@ static void list_config_help(enum show_config_type type)
> struct string_list keys = STRING_LIST_INIT_DUP;
> struct string_list keys_uniq = STRING_LIST_INIT_DUP;
> struct string_list_item *item;
> + struct strbuf sb = STRBUF_INIT;
>
> for (p = config_name_list; *p; p++) {
> const char *var = *p;
> - struct strbuf sb = STRBUF_INIT;
>
> for (e = slot_expansions; e->prefix; e++) {
> What's missing from the context here is that the next line already knows to `strbuf_reset()`. You could do a trick and drop the empty newline here while at it, as that would then make the reset call visible.
Show 13 quoted lines
> @@ -149,7 +149,6 @@ static void list_config_help(enum show_config_type type)
> break;
> }
> }
> - strbuf_release(&sb);
> if (!e->prefix)
> string_list_append(&keys, var);
> }
> @@ -161,10 +160,10 @@ static void list_config_help(enum show_config_type type)
>
> string_list_sort(&keys);
> for (size_t i = 0; i < keys.nr; i++) {
> + strbuf_reset(&sb);Our coding style says that statements should come after variable declarations.
Other than that this patch looks good to me, thanks!
Patrick