From: Michael Haggerty Date: Mon, 10 Aug 2015 03:08:46 GMT Subject: Re: [PATCH 06/14] tempfile: add several functions for creating temporary files Message-ID: <55C815BE.4070309@alum.mit.edu> In-Reply-To: On 06/10/2015 07:48 PM, Junio C Hamano wrote: > Michael Haggerty 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 >> --- >> 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? These are both good points. I will implement them in the upcoming v2. Thanks, Michael -- Michael Haggerty mhagger@alum.mit.edu