Re: [PATCH v2 01/10] pack-write.c: plug a leak in stage_tmp_packfiles()

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2023, 22:00 UTC
Message-ID: <xmqqpm7z7crz.fsf@gitster.g>
In-Reply-To: <c477b754e7ddde0d6e696cfd4027ad88c18aeff3.1681850424.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 11 quoted lines

> The function `stage_tmp_packfiles()` generates a filename to use for
> staging the contents of what will become the pack's ".mtimes" file.
>
> The name is generated in `write_mtimes_file()` and the result is
> returned back to `stage_tmp_packfiles()` which uses it to rename the
> temporary file into place via `rename_tmp_packfiles()`.
>
> `write_mtimes_file()` returns a `const char *`, indicating that callers
> are not expected to free its result (similar to, e.g., `oid_to_hex()`).
> But callers are expected to free its result, so this return type is
> incorrect.

Indeed the string that holds the name of the file returned by write_mtimes_file() is leaking. Does the same logic apply to the returned filename from write_rev_file() and stored in rev_tmp_name that is not freed in stage_tmp_packfiles() in another topic?

Show 18 quoted lines

> @@ -544,7 +544,7 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,
>  			 char **idx_tmp_name)
>  {
>  	const char *rev_tmp_name = NULL;
> -	const char *mtimes_tmp_name = NULL;
> +	char *mtimes_tmp_name = NULL;
>  
>  	if (adjust_shared_perm(pack_tmp_name))
>  		die_errno("unable to make temporary pack file readable");
> @@ -568,6 +568,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,
>  		rename_tmp_packfile(name_buffer, rev_tmp_name, "rev");
>  	if (mtimes_tmp_name)
>  		rename_tmp_packfile(name_buffer, mtimes_tmp_name, "mtimes");
> +
> +	free(mtimes_tmp_name);
>  }
>  
>  void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_sought)