diff mbox series

[2/3] remote.c: don't dereference NULL in freeing loop

Message ID patch-2.3-0e258c230f6-20220607T154520Z-avarab@gmail.com (mailing list archive)
State Accepted
Commit 323822c72be59ce2900cc036c5bad4f10bafbb53
Headers show
Series remote API: fix -fanalyzer-spotted freeing issue | expand

Commit Message

Ævar Arnfjörð Bjarmason June 7, 2022, 3:50 p.m. UTC 
Fix a bug in fd3cb0501e1 (remote: move static variables into
per-repository struct, 2021-11-17) where we'd free(remote->pushurl[i])
after having NULL'd out remote->pushurl. itself. We free
"remote->pushurl" in the next "for"-loop, so doing this appears to
have been a copy/paste error.

Before this change GCC 12's -fanalyzer would correctly note that we'd
dereference NULL in this case, this change fixes that:

	remote.c: In function ‘remote_clear’:
	remote.c:153:17: error: dereference of NULL ‘*remote.pushurl’ [CWE-476] [-Werror=analyzer-null-dereference]
	  153 |                 free((char *)remote->pushurl[i]);
	      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
	      [...]

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano June 7, 2022, 5:23 p.m. UTC | #1 
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Fix a bug in fd3cb0501e1 (remote: move static variables into
> per-repository struct, 2021-11-17) where we'd free(remote->pushurl[i])
> after having NULL'd out remote->pushurl. itself. We free

Wow, that's a bad one.  Why didn't anybody notice this at runtime, I
have to wonder.

>  	for (i = 0; i < remote->url_nr; i++)
>  		free((char *)remote->url[i]);
> -	FREE_AND_NULL(remote->pushurl);
> +	FREE_AND_NULL(remote->url);
>  
>  	for (i = 0; i < remote->pushurl_nr; i++)
>  		free((char *)remote->pushurl[i]);
>  	FREE_AND_NULL(remote->pushurl);

Thanks.
diff mbox series

Patch

diff --git a/remote.c b/remote.c
index 3e75db7bb4f..0b243b090d9 100644
--- a/remote.c
+++ b/remote.c
@@ -147,7 +147,7 @@  static void remote_clear(struct remote *remote)
 
 	for (i = 0; i < remote->url_nr; i++)
 		free((char *)remote->url[i]);
-	FREE_AND_NULL(remote->pushurl);
+	FREE_AND_NULL(remote->url);
 
 	for (i = 0; i < remote->pushurl_nr; i++)
 		free((char *)remote->pushurl[i]);