Am 17.12.21 um 12:26 schrieb Han Xin:
> From: Han Xin <hanxin.hx@xxxxxxxxxxxxxxx>
>
> Fix a strbuf leak in "write_loose_object()" sugguested by
> Ævar Arnfjörð Bjarmason.
>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> Signed-off-by: Han Xin <hanxin.hx@xxxxxxxxxxxxxxx>
> ---
> object-file.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index eb1426f98c..32acf1dad6 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1874,11 +1874,14 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
Relevant context lines:
static struct strbuf tmp_file = STRBUF_INIT;
static struct strbuf filename = STRBUF_INIT;
loose_object_path(the_repository, &filename, oid);
> fd = create_tmpfile(&tmp_file, filename.buf);
> if (fd < 0) {
> if (flags & HASH_SILENT)
> - return -1;
> + ret = -1;
> else if (errno == EACCES)
> - return error(_("insufficient permission for adding an object to repository database %s"), get_object_directory());
> + ret = error(_("insufficient permission for adding an "
> + "object to repository database %s"),
> + get_object_directory());
> else
> - return error_errno(_("unable to create temporary file"));
> + ret = error_errno(_("unable to create temporary file"));
> + goto cleanup;
> }
>
> /* Set it up */
> @@ -1930,7 +1933,11 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
> warning_errno(_("failed utime() on %s"), tmp_file.buf);
> }
>
> - return finalize_object_file(tmp_file.buf, filename.buf);
> + ret = finalize_object_file(tmp_file.buf, filename.buf);
> +cleanup:
> + strbuf_release(&filename);
> + strbuf_release(&tmp_file);
There was no leak before. Both strbufs are static and both functions
they are passed to (loose_object_path() and create_tmpfile()) reset
them first. So while the allocated memory was not released before,
it was reused.
Not sure if making write_loose_object() allocate and release these
buffers on every call has much of a performance impact. The only
reason I can think of for wanting such a change is to get rid of the
static buffers, to allow the function to be used by concurrent
threads.
So I think either keeping the code as-is or also making the strbufs
non-static would be better (but then discussing a possible
performance impact in the commit message would be nice).
> + return ret;
> }
>
> static int freshen_loose_object(const struct object_id *oid)
