From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Mon, 10 Oct 2022 15:08:51 +0000 (-0700)
Subject: smyrna savefiledlg, on_gvprbuttonsave_clicked: fix memory leaks
X-Git-Tag: 7.0.0~8^2~4
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=99cc6ba2ab9d70da3def6833b4fce0cc3f338f75;p=graphviz

smyrna savefiledlg, on_gvprbuttonsave_clicked: fix memory leaks

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, `savefiledlg` 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.

On top of this, `on_gvprbuttonsave_clicked` was never calling `agxbfree` on its
local `xbuf`. By moving to a standard string, we also fix this memory leak.

¹ 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 4c148f298..1b706c9ea 100644
--- a/cmd/smyrna/gui/gui.c
+++ b/cmd/smyrna/gui/gui.c
@@ -145,7 +145,9 @@ int openfiledlg(char **filename) {
     return rv;
 }
 
-int savefiledlg(agxbuf *xbuf) {
+int savefiledlg(char **filename) {
+    assert(filename != NULL);
+
     GtkWidget *dialog;
     int rv;
 
@@ -158,8 +160,7 @@ int savefiledlg(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 f903caad4..c0d44b8ba 100644
--- a/cmd/smyrna/gui/gui.h
+++ b/cmd/smyrna/gui/gui.h
@@ -17,7 +17,6 @@
 #include <glade/glade.h>
 #include "callbacks.h"
 #include <cgraph/cgraph.h>
-#include <cgraph/agxbuf.h>
 
 #define MAXIMUM_WIDGET_COUNT	97
 
@@ -42,6 +41,11 @@
  * \return Non-zero on success.
  */
     int openfiledlg(char **filename);
-/*generic save file dialog*/
-    int savefiledlg(agxbuf * xbuf);
+/** generic save file dialog
+ *
+ * \param [out] filename Selected filename on success. Caller should \p g_free
+ *   this.
+ * \return Non-zero on success.
+ */
+    int savefiledlg(char **filename);
     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 a9542c02f..4e4e8b6ef 100644
--- a/cmd/smyrna/gui/menucallbacks.c
+++ b/cmd/smyrna/gui/menucallbacks.c
@@ -396,15 +396,15 @@ void on_gvprbuttonsave_clicked(GtkWidget * widget, gpointer user_data)
     (void)user_data;
 
     FILE *output_file = NULL;
-    agxbuf xbuf = {0};
     GtkTextBuffer *gtkbuf;	/*GTK buffer from glade GUI */
     char *bf2;
     GtkTextIter startit;
     GtkTextIter endit;
 
-    /*file name should be returned in xbuf */
-    if (savefiledlg(&xbuf)) {
-	output_file = fopen(agxbuse(&xbuf), "w");
+    char *filename = NULL;
+    if (savefiledlg(&filename)) {
+	output_file = fopen(filename, "w");
+	g_free(filename);
 	if (output_file) {
 	    gtkbuf =
 		gtk_text_view_get_buffer((GtkTextView *)