libdocument: Clean up temp file handling

t push origin HEAD:master
Make sure we can cope with not being able to create our temp directory.
Bug #595372.

4 files changed, 202 insertions, 93 deletions

diff --git a/libdocument/ev-attachment.c b/libdocument/ev-attachment.c
index c1d1333..c4dd38d 100644
--- a/libdocument/ev-attachment.c
+++ b/libdocument/ev-attachment.c
@@ -101,7 +101,7 @@ ev_attachment_finalize (GObject *object)
 		attachment->priv->tmp_file = NULL;
 	}
 
-	(* G_OBJECT_CLASS (ev_attachment_parent_class)->finalize) (object);
+	G_OBJECT_CLASS (ev_attachment_parent_class)->finalize (object);
 }
 
 static void
@@ -411,18 +411,19 @@ ev_attachment_open (EvAttachment *attachment,
 		return FALSE;
 	}
 
-	if (attachment->priv->tmp_file &&
-	    g_file_query_exists (attachment->priv->tmp_file, NULL)) {
+	if (attachment->priv->tmp_file) {
 		retval = ev_attachment_launch_app (attachment, screen,
 						   timestamp, error);
 	} else {
-		GFile *tmpdir;
+                char *template;
 		GFile *file;
-		
-		tmpdir = g_file_new_for_path (ev_tmp_dir ());
-		file = g_file_get_child (tmpdir, attachment->priv->name);
 
-		if (ev_attachment_save (attachment, file, error)) {
+                /* FIXMEchpe: convert to filename encoding first! */
+                template = g_strdup_printf ("%s.XXXXXX", ev_attachment_get_name (attachment));
+                file = ev_mkstemp_file (template, error);
+                g_free (template);
+
+		if (file != NULL && ev_attachment_save (attachment, file, error)) {
 			if (attachment->priv->tmp_file)
 				g_object_unref (attachment->priv->tmp_file);
 			attachment->priv->tmp_file = g_object_ref (file);
@@ -432,7 +433,6 @@ ev_attachment_open (EvAttachment *attachment,
 		}
 
 		g_object_unref (file);
-		g_object_unref (tmpdir);
 	}
 
 	return retval;
diff --git a/libdocument/ev-file-helpers.c b/libdocument/ev-file-helpers.c
index 3da4a59..4816db0 100644
--- a/libdocument/ev-file-helpers.c
+++ b/libdocument/ev-file-helpers.c
@@ -1,5 +1,6 @@
 /*
  *  Copyright (C) 2002 Jorn Baayen
+ *  Copyright © 2009 Christian Persch
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -14,58 +15,85 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
- *
- *  $Id$
  */
 
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
+#include <config.h>
 
 #include <stdlib.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <string.h>
+#include <errno.h>
+
 #include <glib.h>
 #include <glib/gstdio.h>
-#include <errno.h>
+#include <glib/gi18n-lib.h>
 
 #include "ev-file-helpers.h"
 
 static gchar *tmp_dir = NULL;
-static gint   count = 0;
 
+/**
+ * ev_dir_ensure_exists:
+ * @dir: the directory name
+ * @mode: permissions to use when creating the directory
+ * @error: a location to store a #GError
+ *
+ * Create @dir recursively with permissions @mode.
+ *
+ * Returns: %TRUE on success, or %FALSE on error with @error filled in
+ */
 gboolean
 ev_dir_ensure_exists (const gchar *dir,
-                      int          mode)
+                      int          mode,
+                      GError     **error)
 {
+        int errsv;
+        char *display_name;
+
+        g_return_val_if_fail (dir != NULL, FALSE);
+
+        errno = 0;
 	if (g_mkdir_with_parents (dir, mode) == 0)
 		return TRUE;
 
-	if (errno == EEXIST)
-		return g_file_test (dir, G_FILE_TEST_IS_DIR);
-	
-	g_warning ("Failed to create directory %s: %s", dir, g_strerror (errno));
+        errsv = errno;
+	if (errsv == EEXIST && g_file_test (dir, G_FILE_TEST_IS_DIR))
+                return TRUE;
+
+        display_name = g_filename_display_name (dir);
+        g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
+                     "Failed to create directory '%s': %s",
+                     display_name, g_strerror (errsv));
+        g_free (display_name);
+
 	return FALSE;
 }
 
-const gchar *
-ev_tmp_dir (void)
+/*
+ * _ev_tmp_dir:
+ * @error: a location to store a #GError
+ *
+ * Returns the tmp directory.
+ *
+ * Returns: the tmp directory, or %NULL with @error filled in if the
+ *   directory could not be created
+ */
+const char *
+_ev_tmp_dir (GError **error)
 {
-	if (tmp_dir == NULL) {
-		gboolean exists;
-		gchar   *dirname, *prgname;
+
+        if (tmp_dir == NULL) {
+                gchar *dirname, *prgname;
 
                 prgname = g_get_prgname ();
-		dirname = g_strdup_printf ("%s-%u", prgname ? prgname : "unknown", getpid ());
-		tmp_dir = g_build_filename (g_get_tmp_dir (),
-					    dirname,
-					    NULL);
-		g_free (dirname);
-
-		exists = ev_dir_ensure_exists (tmp_dir, 0700);
-		g_assert (exists);
-	}
+                dirname = g_strdup_printf ("%s-%u", prgname ? prgname : "unknown", getpid ());
+                tmp_dir = g_build_filename (g_get_tmp_dir (), dirname, NULL);
+                g_free (dirname);
+        }
+
+        if (!ev_dir_ensure_exists (tmp_dir, 0700, error))
+                return NULL;
 
 	return tmp_dir;
 }
@@ -85,47 +113,121 @@ _ev_file_helpers_shutdown (void)
 	tmp_dir = NULL;
 }
 
-GFile *
-ev_tmp_file_get (const gchar *prefix)
+/**
+ * ev_mkstemp:
+ * @template: a template string; must contain 'XXXXXX', but not necessarily as a suffix
+ * @file_name: a location to store the filename of the temp file
+ * @error: a location to store a #GError
+ *
+ * Creates a temp file in the evince temp directory.
+ *
+ * Returns: a file descriptor to the newly created temp file name, or %-1
+ *   on error with @error filled in
+ */
+int
+ev_mkstemp (const char  *template,
+            char       **file_name,
+            GError     **error)
 {
-	gchar *path;
-	GFile *file;
+        const char *tmp;
+        char *name;
+        int fd;
 
-	path = ev_tmp_filename (prefix);
-	file = g_file_new_for_path (path);
-	
-	g_free (path);
-	
-	return file;
+        if ((tmp = _ev_tmp_dir (error)) == NULL)
+              return -1;
+
+        name = g_build_filename (tmp, template, NULL);
+        fd = g_mkstemp (name);
+
+        if (fd == -1) {
+		int errsv = errno;
+
+                g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
+                             _("Failed to create a temporary file: %s"),
+                             g_strerror (errsv));
+
+                g_free (name);
+                return -1;
+        }
+
+        if (file_name)
+                *file_name = name;
+
+        return fd;
 }
 
-gchar * 
-ev_tmp_filename (const gchar *prefix)
+static void
+close_fd_cb (gpointer fdptr)
 {
-	gchar *basename;
-	gchar *filename = NULL;
-
-	do {
-		if (filename != NULL)
-			g_free (filename);
-			
-		basename = g_strdup_printf ("%s-%d",
-					    prefix ? prefix : "document",
-					    count ++);
-		
-		filename = g_build_filename (ev_tmp_dir (),
-					     basename, NULL);
-		
-		g_free (basename);
-	} while (g_file_test (filename, G_FILE_TEST_EXISTS));
-			
-	return filename;
+        int fd = GPOINTER_TO_INT (fdptr);
+
+        close (fd);
+}
+
+/**
+ * ev_mkstemp_file:
+ * @template: a template string; must contain 'XXXXXX', but not necessarily as a suffix
+ * @error: a location to store a #GError
+ *
+ * Creates a temp #GFile in the evince temp directory. See ev_mkstemp() for more information.
+ *
+ * Returns: a newly allocated #GFile for the newly created temp file name, or %NULL
+ *   on error with @error filled in
+ */
+GFile *
+ev_mkstemp_file (const char        *template,
+                 GError           **error)
+{
+        char *file_name;
+        int fd;
+        GFile *file;
+
+        fd = ev_mkstemp (template, &file_name, error);
+        if (fd == -1)
+                return NULL;
+
+        file = g_file_new_for_path (file_name);
+        g_free (file_name);
+
+        g_object_set_data_full (G_OBJECT (file), "ev-mkstemp-fd",
+                                GINT_TO_POINTER (fd), (GDestroyNotify) close_fd_cb);
+
+        return file;
 }
 
-gchar * 
-ev_tmp_directory (const gchar *prefix) 
+/**
+ * ev_mkdtemp:
+ * @template: a template string; must end in 'XXXXXX'
+ * @error: a location to store a #GError
+ *
+ * Creates a temp directory in the evince temp directory.
+ *
+ * Returns: a newly allocated string with the temp directory name, or %NULL
+ *   on error with @error filled in
+ */
+gchar *
+ev_mkdtemp (const char        *template,
+            GError           **error)
 {
-	return ev_tmp_filename (prefix ? prefix : "directory");
+        const char *tmp;
+        char *name;
+
+        if ((tmp = _ev_tmp_dir (error)) == NULL)
+              return NULL;
+
+        name = g_build_filename (tmp, template, NULL);
+        if (mkdtemp (name) == NULL) {
+		int errsv = errno;
+
+                g_set_error (error, G_IO_ERROR, g_io_error_from_errno (errsv),
+                             _("Failed to create a temporary directory: %s"),
+                             g_strerror (errsv));
+
+                g_free (name);
+                return NULL;
+        }
+
+        return name;
 }
 
 /* Remove a local temp file created by evince */
@@ -138,7 +240,7 @@ ev_tmp_filename_unlink (const gchar *filename)
 		return;
 
 	tempdir = g_get_tmp_dir ();
-	if (g_ascii_strncasecmp (filename, tempdir, strlen (tempdir)) == 0) {
+	if (g_str_has_prefix (filename, tempdir) == 0) {
 		g_unlink (filename);
 	}
 }
@@ -326,7 +428,7 @@ compression_run (const gchar       *uri,
 {
 	gchar *argv[N_ARGS];
 	gchar *uri_dst = NULL;
-	gchar *filename, *filename_dst;
+	gchar *filename, *filename_dst = NULL;
 	gchar *cmd;
 	gint   fd, pout;
 	GError *err = NULL;
@@ -349,20 +451,12 @@ compression_run (const gchar       *uri,
 		g_free (cmd);
 		return NULL;
 	}
-	
-	filename_dst = g_build_filename (ev_tmp_dir (), "evinceXXXXXX", NULL);
-	fd = g_mkstemp (filename_dst);
-	if (fd < 0) {
-		int errsv = errno;
 
+        fd = ev_mkstemp ("comp.XXXXXX", &filename_dst, error);
+	if (fd == -1) {
 		g_free (cmd);
 		g_free (filename);
-		g_free (filename_dst);
 
-		g_set_error (error, G_IO_ERROR,
-			     g_io_error_from_errno (errsv),
-			     "Error creating a temporary file: %s",
-			     g_strerror (errsv));
 		return NULL;
 	}
 
diff --git a/libdocument/ev-file-helpers.h b/libdocument/ev-file-helpers.h
index 47a5417..99df4ed 100644
--- a/libdocument/ev-file-helpers.h
+++ b/libdocument/ev-file-helpers.h
@@ -36,18 +36,23 @@ typedef enum {
 	EV_COMPRESSION_GZIP
 } EvCompressionType;
 
-const gchar *ev_tmp_dir               (void);
+const char *_ev_tmp_dir               (GError **error);
 
 void        _ev_file_helpers_init     (void);
 
 void        _ev_file_helpers_shutdown (void);
 
 gboolean     ev_dir_ensure_exists     (const gchar       *dir,
-                                       int                mode);
-
-GFile       *ev_tmp_file_get          (const gchar       *prefix);
-gchar       *ev_tmp_filename          (const char        *prefix);
-gchar       *ev_tmp_directory         (const char        *prefix);
+                                       int                mode,
+                                       GError           **error);
+
+int          ev_mkstemp               (const char        *template,
+                                       char             **file_name,
+                                       GError           **error);
+GFile       *ev_mkstemp_file          (const char        *template,
+                                       GError           **error);
+gchar       *ev_mkdtemp               (const char        *template,
+                                       GError           **error);
 void         ev_tmp_filename_unlink   (const gchar       *filename);
 void         ev_tmp_file_unlink       (GFile             *file);
 void         ev_tmp_uri_unlink        (const gchar       *uri);
diff --git a/libdocument/ev-image.c b/libdocument/ev-image.c
index 4fa2225..572b119 100644
--- a/libdocument/ev-image.c
+++ b/libdocument/ev-image.c
@@ -18,7 +18,10 @@
  */
 
 #include <config.h>
+
 #include <glib/gstdio.h>
+#include <unistd.h>
+
 #include "ev-document-misc.h"
 #include "ev-file-helpers.h"
 #include "ev-image.h"
@@ -132,7 +135,8 @@ ev_image_save_tmp (EvImage   *image,
 		   GdkPixbuf *pixbuf)
 {
 	GError *error = NULL;
-	gchar  *filename;
+	gchar  *filename = NULL;
+        int fd;
 	
 	g_return_val_if_fail (EV_IS_IMAGE (image), NULL);
 	g_return_val_if_fail (GDK_IS_PIXBUF (pixbuf), NULL);
@@ -140,19 +144,28 @@ ev_image_save_tmp (EvImage   *image,
 	if (image->priv->tmp_uri)
 		return image->priv->tmp_uri;
 
-	filename = ev_tmp_filename ("image");
+        if ((fd = ev_mkstemp ("image.XXXXXX", &filename, &error)) == -1)
+                goto had_error;
+
 	gdk_pixbuf_save (pixbuf, filename,
 			 "png", &error,
 			 "compression", "3", NULL);
+        close (fd);
+
 	if (!error) {
-		image->priv->tmp_uri = g_filename_to_uri (filename, NULL, NULL);
+		image->priv->tmp_uri = g_filename_to_uri (filename, NULL, &error);
+                if (image->priv->tmp_uri == NULL)
+                        goto had_error;
+
 		g_free (filename);
 		
 		return image->priv->tmp_uri;
 	}
 
+    had_error:
+
 	/* Erro saving image */
-	g_warning ("%s", error->message);
+	g_warning ("Error saving image: %s", error->message);
 	g_error_free (error);
 	g_free (filename);
 
@@ -166,6 +179,3 @@ ev_image_get_tmp_uri (EvImage *image)
 
 	return image->priv->tmp_uri;
 }
-
-
-