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

From: Junio C Hamano <[email-masked]>
Date: Wed, 10 Jun 2015 10:48:47 -0700

Michael Haggerty <[email-masked]> writes:

> 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 <[email-masked]>
> ---
>  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?