From: Patrick Steinhardt Date: Wed, 04 Mar 2026 07:42:33 GMT Subject: Re: [PATCH v7 01/10] fsmonitor: fix khash memory leak in do_handle_client Message-ID: In-Reply-To: <4d4dec8fa161926e6f6ac822aff0db35353705eb.1772065643.git.gitgitgadget@gmail.com> On Thu, Feb 26, 2026 at 12:27:14AM +0000, Paul Tarjan via GitGitGadget wrote: > diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c > index 242c594646..bc4571938c 100644 > --- a/builtin/fsmonitor--daemon.c > +++ b/builtin/fsmonitor--daemon.c > @@ -671,7 +671,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, > const struct fsmonitor_batch *batch; > struct fsmonitor_batch *remainder = NULL; > intmax_t count = 0, duplicates = 0; > - kh_str_t *shown; > + kh_str_t *shown = NULL; > int hash_ret; > int do_trivial = 0; > int do_flush = 0; > @@ -909,8 +909,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, > total_response_len += payload.len; > } > > - kh_release_str(shown); > - > pthread_mutex_lock(&state->main_lock); > > if (token_data->client_ref_count > 0) > @@ -954,6 +952,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, > trace2_data_intmax("fsmonitor", the_repository, "response/count/duplicates", duplicates); > > cleanup: > + kh_destroy_str(shown); > strbuf_release(&response_token); > strbuf_release(&requested_token_id); > strbuf_release(&payload); Makes sense. If I understood correctly I think we could improve this code to stop using khash directly and instead use a strmap, which has a nicer interface. But that's certainly outside of the scope of this patch series and rather a #leftoverbit. Patrick