Change the buggy "remote_clear()" function to stop pretending to to be able to zero out a "struct remote". Setting "url" and "pushurl" to NULL results in an invalid state unless the corresponding "url_nr" and "pushurl_nr" are also set to zero. In this case however we do not use the "struct remote", so the FREE_AND_NULL() pattern added in fd3cb0501e1 (remote: move static variables into per-repository struct, 2021-11-17) can be replaced with free(). The API was also odd in that remote_state_new() would xmalloc() for us, but the user had to free() it themselves, let's instead change the behavior to have the destructor free() what we malloc() in the constructer. In this case this appears to have been done for consistency with repo_clear(), let's instead have repo_clear() handle the NULL-ing of its "remote_state", and not attempt to reset the structure in remote.c Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> --- remote.c | 14 +++++++------- remote.h | 10 +++++++++- repository.c | 2 +- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/remote.c b/remote.c index 0b243b090d9..c6ce04dacb7 100644 --- a/remote.c +++ b/remote.c @@ -147,15 +147,15 @@ static void remote_clear(struct remote *remote) for (i = 0; i < remote->url_nr; i++) free((char *)remote->url[i]); - FREE_AND_NULL(remote->url); + free(remote->url); for (i = 0; i < remote->pushurl_nr; i++) free((char *)remote->pushurl[i]); - FREE_AND_NULL(remote->pushurl); + free(remote->pushurl); free((char *)remote->receivepack); free((char *)remote->uploadpack); - FREE_AND_NULL(remote->http_proxy); - FREE_AND_NULL(remote->http_proxy_authmethod); + free(remote->http_proxy); + free(remote->http_proxy_authmethod); } static void add_merge(struct branch *branch, const char *name) @@ -2720,12 +2720,12 @@ void remote_state_clear(struct remote_state *remote_state) for (i = 0; i < remote_state->remotes_nr; i++) remote_clear(remote_state->remotes[i]); - FREE_AND_NULL(remote_state->remotes); - remote_state->remotes_alloc = 0; - remote_state->remotes_nr = 0; + free(remote_state->remotes); hashmap_clear_and_free(&remote_state->remotes_hash, struct remote, ent); hashmap_clear_and_free(&remote_state->branches_hash, struct remote, ent); + + free(remote_state); } /* diff --git a/remote.h b/remote.h index dd4402436f1..d91b2b29373 100644 --- a/remote.h +++ b/remote.h @@ -54,9 +54,17 @@ struct remote_state { int initialized; }; -void remote_state_clear(struct remote_state *remote_state); +/** + * xmalloc() a "struct remote_state" and initialize it. The resulting + * data should be free'd with remote_state_clear(). + */ struct remote_state *remote_state_new(void); +/** + * free() the structure returned by remote_state_new(). + */ +void remote_state_clear(struct remote_state *remote_state); + struct remote { struct hashmap_entry ent; diff --git a/repository.c b/repository.c index 5d166b692c8..0a6df6937e4 100644 --- a/repository.c +++ b/repository.c @@ -292,7 +292,7 @@ void repo_clear(struct repository *repo) if (repo->remote_state) { remote_state_clear(repo->remote_state); - FREE_AND_NULL(repo->remote_state); + repo->remote_state = NULL; } repo_clear_path_cache(&repo->cached_paths); -- 2.36.1.1178.g0c3594a0ba5