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

2015-08-09 Thread Michael Haggerty
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
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-10 Thread Junio C Hamano
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?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-06-08 Thread Michael Haggerty
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;
@@ -82,6 +82,59 @@ int create_tempfile(struct tempfile *tempfile, const char 
*path)
return tempfile->fd;
 }
 
+int mks_tempfile_sm(struct tempfile *tempfile,
+   const char *template, int suffixlen, int mode)
+{
+   register_tempfile_object(tempfile, template);
+
+   strbuf_add_absolute_path(&tempfile->filename, template);
+   tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, 
mode);
+   if (tempfile->fd < 0) {
+   strbuf_reset(&tempfile->filename);
+   return -1;
+   }
+   tempfile->owner = getpid();
+   tempfile->active = 1;
+   return tempfile->fd;
+}
+
+int mks_tempfile_tsm(struct tempfile *tempfile,
+const char *template, int suffixlen, int mode)
+{
+   const char *tmpdir;
+
+   register_tempfile_object(tempfile, template);
+
+   tmpdir = getenv("TMPDIR");
+   if (!tmpdir)
+   tmpdir = "/tmp";
+
+   strbuf_addf(&tempfile->filename, "%s/%s", tmpdir, template);
+   tempfile->fd = git_mkstemps_mode(tempfile->filename.buf, suffixlen, 
mode);
+   if (tempfile->fd < 0) {
+   strbuf_reset(&tempfile->filename);
+   return -1;
+   }
+   tempfile->owner = getpid();
+   tempfile->active = 1;
+   return tempfile->fd;
+}
+
+int xmks_tempfile_m(struct tempfile *tempfile, const char *template, int mode)
+{
+   int fd;
+   struct strbuf full_template = STRBUF_INIT;
+
+   strbuf_add_absolute_path(&full_template, template);
+   fd = mks_tempfile_m(tempfile, full_template.buf, mode);
+   if (fd < 0)
+   die_errno("Unable to create temporary file '%s'",
+ full_template.buf);
+
+   strbuf_release(&full_template);
+   return fd;
+}
+
 FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
 {
if (!tempfile->active)
diff --git a/tempfile.h b/tempfile.h
index f83deac..6276156 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -145,6 +145,102 @@ struct tempfile {
  */
 extern int create_tempfile(struct tempfile *tempfile, const char *path);
 
+
+/*
+ * mks_tempfile functions
+ *
+ * The following functions attempt to create and open temporary files
+ * with names derived automatically from a template, in the manner of
+ * mkstemps(), and arrange for them to be deleted if the program ends
+ * before they are deleted explicitly. There is a whole family of such
+ * functions, named according to the following pattern:
+ *
+ * x?mks_tempfile_t?s?m?()
+ *
+ * The optional letters have the following meanings:
+ *
+ *   x - die if the temporary file cannot be created.
+ *
+ *   t - create the temporary file under $TMPDIR (as opposed to
+ *   relative to the current directory). When these variants are
+ *   used, template should be the pattern for the filename alone,
+ *   without a path.
+ *
+ *   s - template includes a suffix that is suffixlen characters long.
+ *
+ *   m - the temporary file should be created with the specified mode
+ *   (otherwise, the mode is set to 0600).
+ *
+ * None of these functions modify template. If the caller wants to
+ * know the (absolute) path of the file that was created, it can be
+ * read from tempfile->filename.
+ *
+ * On success, the functions return a file descriptor that is open for
+ * writing the temporary file. On errors, they return -1 and set errno
+ * appropriately (except for the "x" variants, which die() on errors).
+ */
+
+/* See "mks_tempfile functions" above. */
+extern int mks_tempfile_sm(struct tempfile *tempfile,
+  const char *template, int suffixlen, int mode);
+
+/* See "mks_tempfile functions" above. */
+static inline int mks_tempfile_s(struct tempfile *tempfile,
+const char *template, int suffixlen)
+{
+