[PATCH 3/3] remote API: don't buggily FREE_AND_NULL(), free() instead

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux