[Spice-devel] [spice-gtk v1 10/10] file-transfer: fix leak on _task_get_filename

Victor Toso victortoso at redhat.com
Fri Jul 29 22:26:31 UTC 2016


This fixes the leak of filename. The documentation says that this is
transfer none so Spicy and Virt-Viewer are not trying to free this
string.

The other option would be changing the documentation to transfer full
and fix the applications.

This patch breaks API by using const* to return value and ABI as
client application that was free'ing will need to fix it.

While on it, we use a lot of g_file_get_basename() in the code so I
kept the filename saved as it should not change during the file
transfer.
---
 src/spice-file-transfer-task.c | 20 +++++++++-----------
 src/spice-file-transfer-task.h |  2 +-
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
index fb48b9b..70571f0 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -45,6 +45,7 @@ struct _SpiceFileTransferTask
     gboolean                       completed;
     gboolean                       pending;
     GFile                          *file;
+    gchar                          *filename;
     SpiceMainChannel               *channel;
     GFileInputStream               *file_stream;
     GFileCopyFlags                 flags;
@@ -212,11 +213,9 @@ static void spice_file_transfer_task_read_stream_cb(GObject *source_object,
         gint64 now = g_get_monotonic_time();
 
         if (interval < now - self->last_update) {
-            gchar *basename = g_file_get_basename(self->file);
             self->last_update = now;
             SPICE_DEBUG("read %.2f%% of the file %s",
-                        100.0 * self->read_bytes / self->file_size, basename);
-            g_free(basename);
+                        100.0 * self->read_bytes / self->file_size, self->filename);
         }
     }
 
@@ -246,16 +245,14 @@ static void spice_file_transfer_task_close_stream_cb(GObject      *object,
 
     if (self->error == NULL && spice_util_get_debug()) {
         gint64 now = g_get_monotonic_time();
-        gchar *basename = g_file_get_basename(self->file);
         double seconds = (double) (now - self->start_time) / G_TIME_SPAN_SECOND;
         gchar *file_size_str = g_format_size(self->file_size);
         gchar *transfer_speed_str = g_format_size(self->file_size / seconds);
 
         g_warn_if_fail(self->read_bytes == self->file_size);
         SPICE_DEBUG("transferred file %s of %s size in %.1f seconds (%s/s)",
-                    basename, file_size_str, seconds, transfer_speed_str);
+                    self->filename, file_size_str, seconds, transfer_speed_str);
 
-        g_free(basename);
         g_free(file_size_str);
         g_free(transfer_speed_str);
     }
@@ -540,9 +537,9 @@ void spice_file_transfer_task_cancel(SpiceFileTransferTask *self)
  *
  * Since: 0.31
  **/
-char* spice_file_transfer_task_get_filename(SpiceFileTransferTask *self)
+const char* spice_file_transfer_task_get_filename(SpiceFileTransferTask *self)
 {
-    return g_file_get_basename(self->file);
+    return self->filename;
 }
 
 /*******************************************************************************
@@ -608,6 +605,7 @@ spice_file_transfer_task_dispose(GObject *object)
     g_clear_object(&self->file);
     g_clear_object(&self->file_stream);
     g_clear_error(&self->error);
+    g_clear_pointer(&self->filename, g_free);
 
     G_OBJECT_CLASS(spice_file_transfer_task_parent_class)->dispose(object);
 }
@@ -626,14 +624,14 @@ static void
 spice_file_transfer_task_constructed(GObject *object)
 {
     SpiceFileTransferTask *self = SPICE_FILE_TRANSFER_TASK(object);
+    self->filename = g_file_get_basename(self->file);
 
     if (spice_util_get_debug()) {
-        gchar *basename = g_file_get_basename(self->file);
         self->start_time = g_get_monotonic_time();
         self->last_update = self->start_time;
 
-        SPICE_DEBUG("transfer of file %s has started", basename);
-        g_free(basename);
+        SPICE_DEBUG("transfer of file %s has started", self->filename);
+        g_free(self->filename);
     }
 }
 
diff --git a/src/spice-file-transfer-task.h b/src/spice-file-transfer-task.h
index 4f179fb..43d1d86 100644
--- a/src/spice-file-transfer-task.h
+++ b/src/spice-file-transfer-task.h
@@ -41,7 +41,7 @@ typedef struct _SpiceFileTransferTaskClass SpiceFileTransferTaskClass;
 
 GType spice_file_transfer_task_get_type(void) G_GNUC_CONST;
 
-char* spice_file_transfer_task_get_filename(SpiceFileTransferTask *self);
+const char* spice_file_transfer_task_get_filename(SpiceFileTransferTask *self);
 void spice_file_transfer_task_cancel(SpiceFileTransferTask *self);
 double spice_file_transfer_task_get_progress(SpiceFileTransferTask *self);
 
-- 
2.7.4



More information about the Spice-devel mailing list