Am 03.06.22 um 20:37 schrieb Ævar Arnfjörð Bjarmason:
> 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.
>
> While we're at it let's get rid of the redundant braces per the
> CodingGuidelines, which also serves to show in the diff context that
> we were doing a FREE_AND_NULL(remote->pushurl) afterwards too, let's
> keep that one.
The extended context is helping, but the brace removal makes this change
harder to read.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> remote.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 930fdc9c2f6..61240562df1 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -144,14 +144,10 @@ static void remote_clear(struct remote *remote)
> free((char *)remote->name);
> free((char *)remote->foreign_vcs);
>
> - for (i = 0; i < remote->url_nr; i++) {
> + for (i = 0; i < remote->url_nr; i++)
> free((char *)remote->url[i]);
> - }
> - FREE_AND_NULL(remote->pushurl);
So you remove the redundant release of the pushurl array, but the url
array, which should be freed here after its elements are released, still
leaks. The duplicate FREE_AND_NULL perhaps resulted from a copy&paste
error.
> -
> - for (i = 0; i < remote->pushurl_nr; i++) {
> + for (i = 0; i < remote->pushurl_nr; i++)
> free((char *)remote->pushurl[i]);
> - }
> FREE_AND_NULL(remote->pushurl);
Why set pushurl to NULL after release? This results in an invalid state
unless pushurl_nr und pushurl_alloc are reset to zero. Same goes for
the url array above -- either a simple free(3) call suffices or url_nr
and url_alloc need to be cleared as well.
> free((char *)remote->receivepack);
> free((char *)remote->uploadpack);
