smyrna openfiledlg: fix memory leak

The `gtk_file_chooser_get_filename` docs¹ note:

  Return value
  …
  The currently selected filename, or NULL if no file is selected, or the
  selected file can’t be represented with a local filename. Free with g_free().

  The caller of the method takes ownership of the data, and is responsible for
  freeing it.

Contrary to this, `openfiledlg` was duplicating the pointed to data and then
losing the pointer `gtk_file_chooser_get_filename` returned.

The straightforward fix to this would be to retain the pointer and then `g_free`
it after copying its contents into the buffer `xbuf`. However, we can instead
refactor this function to avoid the copy altogether and simply pass the original
memory back to the caller, making this both a fix and an optimization.

¹ https://docs.gtk.org/gtk3/method.FileChooser.get_filename.html

diff --git a/cmd/smyrna/gui/gui.c b/cmd/smyrna/gui/gui.c
index 3e7cd8d83766e9fe1d6a88c9f33f6d520a67220f..860bd4cfa6e2a74efcde0629483ba2e58d54956c 100644 (file)
--- a/cmd/smyrna/gui/gui.c
+++ b/cmd/smyrna/gui/gui.c
@@ -8,6 +8,7 @@
  * Contributors: Details at https://graphviz.org
  *************************************************************************/
 
+#include <assert.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include "gui.h"
@@ -120,7 +121,9 @@ void show_gui_warning(char *str)
 Generic Open File dialog, if a file is selected and return value is 1, else 0
 file name is copied to char* filename,which should be allocated before using the function
 */
-int openfiledlg(agxbuf *xbuf) {
+int openfiledlg(char **filename) {
+    assert(filename != NULL);
+
     GtkWidget *dialog;
     int rv;
 
@@ -133,8 +136,7 @@ int openfiledlg(agxbuf *xbuf) {
                                         GTK_RESPONSE_ACCEPT, NULL);
 
     if (gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
-       agxbput(xbuf,
-               gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog)));
+       *filename = gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
        rv = 1;
     } else
        rv = 0;

diff --git a/cmd/smyrna/gui/gui.h b/cmd/smyrna/gui/gui.h
index 112b5aa069a6667d7ebe4f6229a5cc675d8ff30b..fff7e412f6da66295d70e34ea2e609ff22e7997d 100644 (file)
--- a/cmd/smyrna/gui/gui.h
+++ b/cmd/smyrna/gui/gui.h
@@ -35,8 +35,13 @@
     void Color_Widget_bg(char *colorstring, GtkWidget * widget);
 /*generic warning pop up*/
     void show_gui_warning(char *str);
-/*generic open file dialog*/
-    int openfiledlg(agxbuf * xbuf);
+/** generic open file dialog
+ *
+ * \param [out] filename Selected filename on success. Caller should \p g_free
+ *   this.
+ * \return Non-zero on success.
+ */
+    int openfiledlg(char **filename);
 /*generic save file dialog*/
     int savefiledlg(int filtercnt, char **filters, agxbuf * xbuf);
     void append_textview(GtkTextView * textv, const char *s, size_t bytes);

diff --git a/cmd/smyrna/gui/menucallbacks.c b/cmd/smyrna/gui/menucallbacks.c
index d55649151a6d775ca99dfccb8d3724bb21954d42..bf8a3eb68e1e001646dfc4854f5de5c9d43a67f1 100644 (file)
--- a/cmd/smyrna/gui/menucallbacks.c
+++ b/cmd/smyrna/gui/menucallbacks.c
@@ -20,6 +20,7 @@
 #include <cgraph/agxbuf.h>
 #include <assert.h>
 #include <ctype.h>
+#include <glib.h>
 #include  "frmobjectui.h"
 
 void mAttributesSlot(GtkWidget * widget, gpointer user_data)
@@ -360,9 +361,10 @@ void on_gvprbuttonload_clicked(GtkWidget * widget, gpointer user_data)
 
     agxbinit(&xbuf, SMALLBUF, xbuffer);
 
-    /*file name should be returned in xbuf */
-    if (openfiledlg(&xbuf)) {
-       input_file = fopen(agxbuse(&xbuf), "r");
+    char *filename = NULL;
+    if (openfiledlg(&filename)) {
+       input_file = fopen(filename, "r");
+       g_free(filename);
        if (input_file) {
            while (fgets(buf, BUFSIZ, input_file))
                agxbput(&xbuf, buf);