From 227c6b88367c3b492e9dd07158a7e05b3a06344f Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Thu, 20 Aug 2020 13:41:38 +0200 Subject: [PATCH 1/2] screenshot: Remove '/' in screenshot filenames Make sure we don't use a slash in the pattern for screenshots or we wouldn't be able to save the screenshot at all. Totem-WARNING **: Could not find a valid location to save the screenshot: Failed to find a valid place to save Closes: #427 --- .../screenshot/totem-screenshot-plugin.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/plugins/screenshot/totem-screenshot-plugin.c b/src/plugins/screenshot/totem-screenshot-plugin.c index 2d488b542..6808b3443 100644 --- a/src/plugins/screenshot/totem-screenshot-plugin.c +++ b/src/plugins/screenshot/totem-screenshot-plugin.c @@ -200,6 +200,16 @@ flash_area (GtkWidget *widget) NULL); } +static char * +escape_video_name (const char *orig) +{ + g_auto(GStrv) elems = NULL; + + /* '/' can't be in a filename */ + elems = g_strsplit (orig, "/", -1); + return g_strjoinv ("–", elems); +} + static void take_screenshot_action_cb (GSimpleAction *action, GVariant *parameter, @@ -209,7 +219,8 @@ take_screenshot_action_cb (GSimpleAction *action, GdkPixbuf *pixbuf; GError *err = NULL; ScreenshotSaveJob *job; - char *video_name; + g_autofree char *video_name = NULL; + g_autofree char *escaped_video_name = NULL; if (bacon_video_widget_get_logo_mode (priv->bvw) != FALSE) return; @@ -232,14 +243,13 @@ take_screenshot_action_cb (GSimpleAction *action, } video_name = totem_object_get_short_title (self->priv->totem); + escaped_video_name = escape_video_name (video_name); job = g_slice_new (ScreenshotSaveJob); job->plugin = self; job->pixbuf = pixbuf; - screenshot_build_filename_async (NULL, video_name, screenshot_name_ready_cb, job); - - g_free (video_name); + screenshot_build_filename_async (NULL, escaped_video_name, screenshot_name_ready_cb, job); } static void -- GitLab From c542bfe80c6047051c2c9b852e1b870ad6b71ce0 Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Thu, 20 Aug 2020 14:22:06 +0200 Subject: [PATCH 2/2] 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 @@ -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__ */ -- GitLab