Re: [PATCH 06/14] tempfile: add several functions for creating temporary files

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 10, 2015, 17:48 UTC
Message-ID: <xmqqmw07laio.fsf@gitster.dls.corp.google.com>
In-Reply-To: <a922fa6cfcc948d541b638d99e2413865ff051e2.1433751986.git.mhagger@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 27 quoted lines

> Add several functions for creating temporary files with
> automatically-generated names, analogous to mkstemps(), but also
> arranging for the files to be deleted on program exit.
>
> The functions are named according to a pattern depending how they
> operate. They will be used to replace many places in the code where
> temporary files are created and cleaned up ad-hoc.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
>  tempfile.c | 55 ++++++++++++++++++++++++++++++++++-
>  tempfile.h | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 150 insertions(+), 1 deletion(-)
>
> diff --git a/tempfile.c b/tempfile.c
> index f76bc07..890075f 100644
> --- a/tempfile.c
> +++ b/tempfile.c
> @@ -48,7 +48,7 @@ static void register_tempfile_object(struct tempfile *tempfile, const char *path
>  		tempfile->fp = NULL;
>  		tempfile->active = 0;
>  		tempfile->owner = 0;
> -		strbuf_init(&tempfile->filename, strlen(path));
> +		strbuf_init(&tempfile->filename, 0);
>  		tempfile->next = tempfile_list;
>  		tempfile_list = tempfile;
>  		tempfile->on_list = 1;

This probably could have been part of the previous step. Regardless of where in the patch series this change is done, I think it makes sense, as this function does not even know how long the final filename would be, and strlen(path) is almost always wrong as path is likely to be relative.

I notice this change makes "path" almost unused in this function, and the only remaining use is for assert(!tempfile->filename.len). Perhaps it is not worth passing the "path" parameter?