From daf31d56bc3d7ce8b75373b816c3eec194077d4e Mon Sep 17 00:00:00 2001
From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Sun, 9 Oct 2022 21:37:45 -0700
Subject: [PATCH] smyrna openfiledlg: fix memory leak
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

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
---
 cmd/smyrna/gui/gui.c           | 8 +++++---
 cmd/smyrna/gui/gui.h           | 9 +++++++--
 cmd/smyrna/gui/menucallbacks.c | 8 +++++---
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/cmd/smyrna/gui/gui.c b/cmd/smyrna/gui/gui.c
index 3e7cd8d83..860bd4cfa 100644
--- 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 112b5aa06..fff7e412f 100644
--- 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 d55649151..bf8a3eb68 100644
--- 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);
-- 
2.40.0