From 946405341ffc12fa6433dab0900da3cff9b1d629 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 26 Nov 2017 14:37:38 +0000 Subject: [PATCH] Fix a cleanup issue in dlgparam_destroy. If a dialog box is destroyed by the program before the user has pressed one of the result-delivering buttons - e.g. because the parent window closes so the dialog is no longer relevant to anything anyway - then dlgparam_destroy would never call the client code's provided callback. That makes sense in terms of the callback wanting to _take action_ based on the result of the dialog box, but it ignores the possibility that the callback may simply need to free its own context structure. So now dlgparam_destroy always calls the client's callback, even if the result it passes is negative (meaning 'the user never got round to pressing any of the dialog-ending buttons'), and all the existing client callbacks handle the negative-result case by doing nothing except freeing any allocated memory they might have. --- unix/gtkapp.c | 4 ++-- unix/gtkdlg.c | 3 +-- unix/gtkmain.c | 4 ++-- unix/gtkwin.c | 7 +++++++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/unix/gtkapp.c b/unix/gtkapp.c index e5cc6c6f..d7add6d1 100644 --- a/unix/gtkapp.c +++ b/unix/gtkapp.c @@ -192,9 +192,9 @@ static void post_initial_config_box(void *vctx, int result) { Conf *conf = (Conf *)vctx; - if (result) { + if (result > 0) { new_session_window(conf, NULL); - } else { + } else if (result == 0) { conf_free(conf); g_application_release(G_APPLICATION(app)); } diff --git a/unix/gtkdlg.c b/unix/gtkdlg.c index 9e5ebf20..35973d9a 100644 --- a/unix/gtkdlg.c +++ b/unix/gtkdlg.c @@ -3316,8 +3316,7 @@ GtkWidget *create_config_box(const char *title, Conf *conf, static void dlgparam_destroy(GtkWidget *widget, gpointer data) { struct dlgparam *dp = (struct dlgparam *)data; - if (dp->retval >= 0) - dp->after(dp->afterctx, dp->retval); + dp->after(dp->afterctx, dp->retval); dlg_cleanup(dp); ctrl_free_box(dp->ctrlbox); #if GTK_CHECK_VERSION(2,0,0) diff --git a/unix/gtkmain.c b/unix/gtkmain.c index a70ceb5b..751a80ff 100644 --- a/unix/gtkmain.c +++ b/unix/gtkmain.c @@ -551,9 +551,9 @@ static void post_initial_config_box(void *vctx, int result) *(struct post_initial_config_box_ctx *)vctx; sfree(vctx); - if (result) { + if (result > 0) { new_session_window(ctx.conf, ctx.geometry_string); - } else { + } else if (result == 0) { /* In this main(), which only runs one session in total, a * negative result from the initial config box means we simply * terminate. */ diff --git a/unix/gtkwin.c b/unix/gtkwin.c index 046cf09c..88b438a5 100644 --- a/unix/gtkwin.c +++ b/unix/gtkwin.c @@ -4016,6 +4016,13 @@ static void after_change_settings_dialog(void *vctx, int retval) sfree(vctx); /* we've copied this already */ + if (retval < 0) { + /* If the dialog box was aborted without giving a result + * (probably because the whole session window closed), we have + * nothing further to do. */ + return; + } + assert(lenof(ww) == NCFGCOLOURS); inst->reconfigure_dialog = NULL;