[totem/wip/hadess/screenshot-slash: 2/2] screenshot: Fix critical in some circumstances

[Bastien Nocera <hadess src gnome org>]

commit c542bfe80c6047051c2c9b852e1b870ad6b71ce0
Author: Bastien Nocera <hadess hadess net>
Date:   Thu Aug 20 14:22:06 2020 +0200

    screenshot: Fix critical in some circumstances
    
    Update filename builder helpers from gnome-screenshot, to fix critical
    when screenshot_build_filename_async() fails.
    
    GLib-GIO-CRITICAL **: g_task_return_pointer: assertion '!task->ever_returned' failed

 .../screenshot/screenshot-filename-builder.c       | 168 +++++++--------------
 .../screenshot/screenshot-filename-builder.h       |   7 +-
 2 files changed, 53 insertions(+), 122 deletions(-)
---
diff --git a/src/plugins/screenshot/screenshot-filename-builder.c 
b/src/plugins/screenshot/screenshot-filename-builder.c
index eb3a0c19e..49f9dda64 100644
--- a/src/plugins/screenshot/screenshot-filename-builder.c
+++ b/src/plugins/screenshot/screenshot-filename-builder.c
@@ -14,7 +14,7 @@
  *
  * 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
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
  * USA
  *
  * 28th June 2012: Bastien Nocera: Add exception clause.
@@ -37,7 +37,7 @@ typedef enum
   NUM_TESTS
 } TestType;
 
-typedef struct 
+typedef struct
 {
   char *base_paths[NUM_TESTS];
   char *screenshot_origin;
@@ -49,13 +49,14 @@ typedef struct
 static char *
 expand_initial_tilde (const char *path)
 {
-  char *slash_after_user_name, *user_name;
+  g_autofree gchar *user_name = NULL;
+  char *slash_after_user_name;
   struct passwd *passwd_file_entry;
 
   if (path[1] == '/' || path[1] == '\0') {
     return g_build_filename (g_get_home_dir (), &path[1], NULL);
   }
-  
+
   slash_after_user_name = strchr (&path[1], '/');
   if (slash_after_user_name == NULL) {
     user_name = g_strdup (&path[1]);
@@ -64,12 +65,11 @@ expand_initial_tilde (const char *path)
                            slash_after_user_name - &path[1]);
   }
   passwd_file_entry = getpwnam (user_name);
-  g_free (user_name);
-  
+
   if (passwd_file_entry == NULL || passwd_file_entry->pw_dir == NULL) {
     return g_strdup (path);
   }
-  
+
   return g_strconcat (passwd_file_entry->pw_dir,
                       slash_after_user_name,
                       NULL);
@@ -90,36 +90,26 @@ get_default_screenshot_dir (void)
 static gchar *
 sanitize_save_directory (const gchar *save_dir)
 {
-  gchar *retval = g_strdup (save_dir);
-
   if (save_dir == NULL)
     return NULL;
 
   if (save_dir[0] == '~')
-    {
-      char *tmp = expand_initial_tilde (save_dir);
-      g_free (retval);
-      retval = tmp;
-    }
-  else if (strstr (save_dir, "://") != NULL)
-    {
-      GFile *file;
+    return expand_initial_tilde (save_dir);
 
-      g_free (retval);
-      file = g_file_new_for_uri (save_dir);
-      retval = g_file_get_path (file);
-      g_object_unref (file);
+  if (strstr (save_dir, "://") != NULL)
+    {
+      g_autoptr(GFile) file = g_file_new_for_uri (save_dir);
+      return g_file_get_path (file);
     }
 
-  return retval;
+  return g_strdup (save_dir);
 }
 
 static char *
 build_path (AsyncExistenceJob *job)
 {
+  g_autofree gchar *file_name = NULL, *origin = NULL;
   const gchar *base_path;
-  char *retval, *file_name;
-  char *origin;
 
   base_path = job->base_paths[job->type];
 
@@ -129,11 +119,8 @@ build_path (AsyncExistenceJob *job)
 
   if (job->screenshot_origin == NULL)
     {
-      GDateTime *d;
-
-      d = g_date_time_new_now_local ();
-      origin = g_date_time_format (d, "%Y-%m-%d %H:%M:%S");
-      g_date_time_unref (d);
+      g_autoptr(GDateTime) d = g_date_time_new_now_local ();
+      origin = g_date_time_format (d, "%Y-%m-%d %H-%M-%S");
     }
   else
     origin = g_strdup (job->screenshot_origin);
@@ -152,11 +139,7 @@ build_path (AsyncExistenceJob *job)
       file_name = g_strdup_printf (_("Screenshot from %s - %d.png"), origin, job->iteration);
     }
 
-  retval = g_build_filename (base_path, file_name, NULL);
-  g_free (file_name);
-  g_free (origin);
-
-  return retval;
+  return g_build_filename (base_path, file_name, NULL);
 }
 
 static void
@@ -195,114 +178,66 @@ try_check_file (GTask *task,
                 GCancellable *cancellable)
 {
   AsyncExistenceJob *job = data;
-  GFile *file;
-  GFileInfo *info;
-  GError *error;
-  char *path, *retval;
 
-retry:
-  error = NULL;
-  path = build_path (job);
-
-  if (path == NULL)
+  while (TRUE)
     {
-      (job->type)++;
-      goto retry;
-    }
+      g_autoptr(GError) error = NULL;
+      g_autoptr(GFile) file = NULL;
+      g_autoptr(GFileInfo) info = NULL;
+      g_autofree gchar *path = build_path (job);
 
-  file = g_file_new_for_path (path);
-  info = g_file_query_info (file, G_FILE_ATTRIBUTE_STANDARD_TYPE,
-                           G_FILE_QUERY_INFO_NONE, cancellable, &error);
-  if (info != NULL)
-    {
-      /* file already exists, iterate again */
-      g_object_unref (info);
-      g_object_unref (file);
-      g_free (path);
+      if (path == NULL)
+        {
+          (job->type)++;
+          continue;
+        }
 
-      (job->iteration)++;
+      file = g_file_new_for_path (path);
+      info = g_file_query_info (file, G_FILE_ATTRIBUTE_STANDARD_TYPE,
+                                G_FILE_QUERY_INFO_NONE, cancellable, &error);
+      if (info != NULL)
+        {
+          /* file already exists, iterate again */
+          (job->iteration)++;
+          continue;
+        }
 
-      goto retry;
-    }
-  else
-    {
       /* see the error to check whether the location is not accessible
        * or the file does not exist.
        */
-      if (error->code == G_IO_ERROR_NOT_FOUND)
+      if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
         {
-          GFile *parent;
+          g_autoptr(GFile) parent = g_file_get_parent (file);
 
-          /* if the parent directory doesn't exist as well, forget the saved
+          /* if the parent directory doesn't exist as well, we'll forget the saved
            * directory and treat this as a generic error.
            */
-
-          parent = g_file_get_parent (file);
-
-          if (!g_file_query_exists (parent, NULL))
-            {
-              if (!prepare_next_cycle (job))
-                {
-                  retval = NULL;
-
-                  g_object_unref (parent);
-                  goto out;
-                }
-
-              g_object_unref (file);
-              g_object_unref (parent);
-              goto retry;
-            }
-          else
+          if (g_file_query_exists (parent, NULL))
             {
-              retval = path;
-
-              g_object_unref (parent);
-              goto out;
+              g_task_return_pointer (task, g_steal_pointer (&path), NULL);
+              return;
             }
         }
-      else
-        {
-          /* another kind of error, assume this location is not
-           * accessible.
-           */
-          g_free (path);
 
-          if (prepare_next_cycle (job))
-            {
-              g_error_free (error);
-              g_object_unref (file);
-              goto retry;
-            }
-          else
-            {
-              retval = NULL;
-              goto out;
-            }
+      if (!prepare_next_cycle (job))
+        {
+          g_task_return_new_error (task,
+                                   G_IO_ERROR,
+                                   G_IO_ERROR_FAILED,
+                                   "%s", "Failed to find a valid place to save");
+          return;
         }
     }
-
-out:
-  g_error_free (error);
-  g_object_unref (file);
-
-  if (retval == NULL)
-    g_task_return_new_error (task,
-                             G_IO_ERROR,
-                             G_IO_ERROR_FAILED,
-                             "%s", "Failed to find a valid place to save");
-
-  g_task_return_pointer (task, retval, NULL);
 }
 
 void
 screenshot_build_filename_async (const char *save_dir,
-                                const char *screenshot_origin,
+                                 const char *screenshot_origin,
                                  GAsyncReadyCallback callback,
                                  gpointer user_data)
 {
   AsyncExistenceJob *job;
-  GTask *task;
+  g_autoptr(GTask) task = NULL;
 
   job = g_slice_new0 (AsyncExistenceJob);
 
@@ -318,7 +253,6 @@ screenshot_build_filename_async (const char *save_dir,
   g_task_set_task_data (task, job, (GDestroyNotify) async_existence_job_free);
 
   g_task_run_in_thread (task, try_check_file);
-  g_object_unref (task);
 }
 
 gchar *
diff --git a/src/plugins/screenshot/screenshot-filename-builder.h 
b/src/plugins/screenshot/screenshot-filename-builder.h
index e70608143..5543fccc7 100644
--- a/src/plugins/screenshot/screenshot-filename-builder.h
+++ b/src/plugins/screenshot/screenshot-filename-builder.h
@@ -14,15 +14,14 @@
  *
  * 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
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
  * USA
  *
  * 28th June 2012: Bastien Nocera: Add exception clause.
  * See license_change file for details.
  */
 
-#ifndef __SCREENSHOT_FILENAME_BUILDER_H__
-#define __SCREENSHOT_FILENAME_BUILDER_H__
+#pragma once
 
 #include <gio/gio.h>
 
@@ -32,5 +31,3 @@ void screenshot_build_filename_async (const char *save_dir,
                                       gpointer user_data);
 gchar *screenshot_build_filename_finish (GAsyncResult *result,
                                          GError **error);
-
-#endif /* __SCREENSHOT_FILENAME_BUILDER_H__ */