Move our DialogBox wrapper into windows/utils.

It's self-contained enough not to really need to live in dialog.c as a
set of static functions. Also, moving it means we can isolate the
implementation details - which also makes it easy to change them.

One such change is that I've added the ability to bake a context
pointer into the dialog - unused so far, but it will be shortly.

(Also, while I'm here, renamed the functions so they sound more as if
they're adding features than working around bugs - not to mention not
imputing mental illness to the usual versions.)

windows/CMakeLists.txt

@@ -30,6 +30,7 @@ add_sources_from_current_dir(utils
   utils/request_file.c
   utils/screenshot.c
   utils/security.c
+  utils/shinydialogbox.c
   utils/split_into_argv.c
   utils/version.c
   utils/win_strerror.c

windows/dialog.c

@@ -255,57 +255,6 @@ static INT_PTR CALLBACK AboutProc(HWND hwnd, UINT msg,
     return 0;
 }
 
-static int SaneDialogBox(HINSTANCE hinst,
-                         LPCTSTR tmpl,
-                         HWND hwndparent,
-                         DLGPROC lpDialogFunc)
-{
-    WNDCLASS wc;
-    HWND hwnd;
-    MSG msg;
-    int flags;
-    int ret;
-    int gm;
-
-    wc.style = CS_DBLCLKS | CS_SAVEBITS | CS_BYTEALIGNWINDOW;
-    wc.lpfnWndProc = DefDlgProc;
-    wc.cbClsExtra = 0;
-    wc.cbWndExtra = DLGWINDOWEXTRA + 2*sizeof(LONG_PTR);
-    wc.hInstance = hinst;
-    wc.hIcon = NULL;
-    wc.hCursor = LoadCursor(NULL, IDC_ARROW);
-    wc.hbrBackground = (HBRUSH) (COLOR_BACKGROUND +1);
-    wc.lpszMenuName = NULL;
-    wc.lpszClassName = "PuTTYConfigBox";
-    RegisterClass(&wc);
-
-    hwnd = CreateDialog(hinst, tmpl, hwndparent, lpDialogFunc);
-
-    SetWindowLongPtr(hwnd, BOXFLAGS, 0); /* flags */
-    SetWindowLongPtr(hwnd, BOXRESULT, 0); /* result from SaneEndDialog */
-
-    while ((gm=GetMessage(&msg, NULL, 0, 0)) > 0) {
-        flags=GetWindowLongPtr(hwnd, BOXFLAGS);
-        if (!(flags & DF_END) && !IsDialogMessage(hwnd, &msg))
-            DispatchMessage(&msg);
-        if (flags & DF_END)
-            break;
-    }
-
-    if (gm == 0)
-        PostQuitMessage(msg.wParam); /* We got a WM_QUIT, pass it on */
-
-    ret=GetWindowLongPtr(hwnd, BOXRESULT);
-    DestroyWindow(hwnd);
-    return ret;
-}
-
-static void SaneEndDialog(HWND hwnd, int ret)
-{
-    SetWindowLongPtr(hwnd, BOXRESULT, ret);
-    SetWindowLongPtr(hwnd, BOXFLAGS, DF_END);
-}
-
 /*
  * Null dialog procedure.
  */
@@ -395,8 +344,8 @@ const char *dialog_box_demo_screenshot_filename = NULL;
  * (Being a dialog procedure, in general it returns 0 if the default
  * dialog processing should be performed, and 1 if it should not.)
  */
-static INT_PTR CALLBACK GenericMainDlgProc(HWND hwnd, UINT msg,
+static INT_PTR GenericMainDlgProc(HWND hwnd, UINT msg, WPARAM wParam,
-                                           WPARAM wParam, LPARAM lParam)
+                                  LPARAM lParam, void *ctx)
 {
     const int DEMO_SCREENSHOT_TIMER_ID = 1230;
     HWND hw, treeview;
@@ -581,7 +530,7 @@ static INT_PTR CALLBACK GenericMainDlgProc(HWND hwnd, UINT msg,
             if (err)
                 MessageBox(hwnd, err, "Demo screenshot failure",
                            MB_OK | MB_ICONERROR);
-            SaneEndDialog(hwnd, 0);
+            ShinyEndDialog(hwnd, 0);
         }
         return 0;
       case WM_LBUTTONUP:
@@ -591,7 +540,7 @@ static INT_PTR CALLBACK GenericMainDlgProc(HWND hwnd, UINT msg,
          */
         ReleaseCapture();
         if (dp.ended)
-            SaneEndDialog(hwnd, dp.endresult ? 1 : 0);
+            ShinyEndDialog(hwnd, dp.endresult ? 1 : 0);
         break;
       case WM_NOTIFY:
         if (LOWORD(wParam) == IDCX_TREEVIEW &&
@@ -657,7 +606,7 @@ static INT_PTR CALLBACK GenericMainDlgProc(HWND hwnd, UINT msg,
         if (GetWindowLongPtr(hwnd, GWLP_USERDATA) == 1) {
             ret = winctrl_handle_command(&dp, msg, wParam, lParam);
             if (dp.ended && GetCapture() != hwnd)
-                SaneEndDialog(hwnd, dp.endresult ? 1 : 0);
+                ShinyEndDialog(hwnd, dp.endresult ? 1 : 0);
         } else
             ret = 0;
         return ret;
@@ -668,7 +617,7 @@ static INT_PTR CALLBACK GenericMainDlgProc(HWND hwnd, UINT msg,
         break;
       case WM_CLOSE:
         quit_help(hwnd);
-        SaneEndDialog(hwnd, 0);
+        ShinyEndDialog(hwnd, 0);
         return 0;
 
         /* Grrr Explorer will maximize Dialogs! */
@@ -729,9 +678,8 @@ bool do_config(Conf *conf)
     dlg_auto_set_fixed_pitch_flag(&dp);
     dp.shortcuts['g'] = true;          /* the treeview: `Cate&gory' */
 
-    ret =
+    ret = ShinyDialogBox(hinst, MAKEINTRESOURCE(IDD_MAINBOX), "PuTTYConfigBox",
-        SaneDialogBox(hinst, MAKEINTRESOURCE(IDD_MAINBOX), NULL,
+                         NULL, GenericMainDlgProc, NULL);
-                  GenericMainDlgProc);
 
     ctrl_free_box(ctrlbox);
     winctrl_cleanup(&ctrls_panel);
@@ -764,8 +712,8 @@ bool do_reconfig(HWND hwnd, Conf *conf, int protcfginfo)
     dlg_auto_set_fixed_pitch_flag(&dp);
     dp.shortcuts['g'] = true;          /* the treeview: `Cate&gory' */
 
-    ret = SaneDialogBox(hinst, MAKEINTRESOURCE(IDD_MAINBOX), NULL,
+    ret = ShinyDialogBox(hinst, MAKEINTRESOURCE(IDD_MAINBOX), "PuTTYConfigBox",
-                  GenericMainDlgProc);
+                         NULL, GenericMainDlgProc, NULL);
 
     ctrl_free_box(ctrlbox);
     winctrl_cleanup(&ctrls_base);

windows/platform.h

@@ -109,9 +109,11 @@ static inline uintmax_t strtoumax(const char *nptr, char **endptr, int base)
 { return _strtoui64(nptr, endptr, base); }
 #endif
 
-#define BOXFLAGS DLGWINDOWEXTRA
+typedef INT_PTR (*ShinyDlgProc)(HWND hwnd, UINT msg, WPARAM wParam,
-#define BOXRESULT (DLGWINDOWEXTRA + sizeof(LONG_PTR))
+                                LPARAM lParam, void *ctx);
-#define DF_END 0x0001
+int ShinyDialogBox(HINSTANCE hinst, LPCTSTR tmpl, const char *winclass,
+                   HWND hwndparent, ShinyDlgProc proc, void *ctx);
+void ShinyEndDialog(HWND hwnd, int ret);
 
 #ifndef __WINE__
 /* Up-to-date Windows headers warn that the unprefixed versions of

windows/utils/shinydialogbox.c

@@ -0,0 +1,111 @@
+/*
+ * PuTTY's own reimplementation of DialogBox() and EndDialog() which
+ * provide extra capabilities.
+ *
+ * Originally introduced in 2003 to work around a problem with our
+ * message loops, in which keystrokes pressed in the 'Change Settings'
+ * dialog in mid-session would _also_ be delivered to the main
+ * terminal window.
+ *
+ * But once we had our own wrapper it was convenient to put further
+ * things into it. In particular, this system allows you to provide a
+ * context pointer at setup time that's easy to retrieve from the
+ * window procedure.
+ */
+
+#include "putty.h"
+
+struct ShinyDialogBoxState {
+    bool ended;
+    int result;
+    ShinyDlgProc proc;
+    void *ctx;
+};
+
+/*
+ * For use in re-entrant calls to the dialog procedure from
+ * CreateDialog itself, temporarily store the state pointer that we
+ * won't store in the usual window-memory slot until later.
+ *
+ * I don't _intend_ that this system will need to be used in multiple
+ * threads at all, let alone concurrently, but just in case, declaring
+ * sdb_tempstate as thread-local will protect against that possibility.
+ */
+static __declspec(thread) struct ShinyDialogBoxState *sdb_tempstate;
+
+static inline struct ShinyDialogBoxState *ShinyDialogGetState(HWND hwnd)
+{
+    if (sdb_tempstate)
+        return sdb_tempstate;
+    return (struct ShinyDialogBoxState *)GetWindowLongPtr(
+        hwnd, DLGWINDOWEXTRA);
+}
+
+static INT_PTR CALLBACK ShinyRealDlgProc(HWND hwnd, UINT msg,
+                                         WPARAM wParam, LPARAM lParam)
+{
+    struct ShinyDialogBoxState *state = ShinyDialogGetState(hwnd);
+    return state->proc(hwnd, msg, wParam, lParam, state->ctx);
+}
+
+int ShinyDialogBox(HINSTANCE hinst, LPCTSTR tmpl, const char *winclass,
+                   HWND hwndparent, ShinyDlgProc proc, void *ctx)
+{
+    /*
+     * Register the window class ourselves in such a way that we
+     * allocate an extra word of window memory to store the state
+     * pointer.
+     *
+     * It would be nice in principle to load the dialog template
+     * resource and dig the class name out of it. But DLGTEMPLATEEX is
+     * a nasty variable-layout structure not declared conveniently in
+     * a header file, so I think that's too much effort. Callers of
+     * this function will just have to provide the right window class
+     * name to match their template resource via the 'winclass'
+     * parameter.
+     */
+    WNDCLASS wc;
+    wc.style = CS_DBLCLKS | CS_SAVEBITS | CS_BYTEALIGNWINDOW;
+    wc.lpfnWndProc = DefDlgProc;
+    wc.cbClsExtra = 0;
+    wc.cbWndExtra = DLGWINDOWEXTRA + sizeof(LONG_PTR);
+    wc.hInstance = hinst;
+    wc.hIcon = NULL;
+    wc.hCursor = LoadCursor(NULL, IDC_ARROW);
+    wc.hbrBackground = (HBRUSH) (COLOR_BACKGROUND +1);
+    wc.lpszMenuName = NULL;
+    wc.lpszClassName = winclass;
+    RegisterClass(&wc);
+
+    struct ShinyDialogBoxState state[1];
+    state->ended = false;
+    state->proc = proc;
+    state->ctx = ctx;
+
+    sdb_tempstate = state;
+    HWND hwnd = CreateDialog(hinst, tmpl, hwndparent, ShinyRealDlgProc);
+    SetWindowLongPtr(hwnd, DLGWINDOWEXTRA, (LONG_PTR)state);
+    sdb_tempstate = NULL;
+
+    MSG msg;
+    int gm;
+    while ((gm = GetMessage(&msg, NULL, 0, 0)) > 0) {
+        if (!state->ended && !IsDialogMessage(hwnd, &msg))
+            DispatchMessage(&msg);
+        if (state->ended)
+            break;
+    }
+
+    if (gm == 0)
+        PostQuitMessage(msg.wParam); /* We got a WM_QUIT, pass it on */
+
+    DestroyWindow(hwnd);
+    return state->result;
+}
+
+void ShinyEndDialog(HWND hwnd, int ret)
+{
+    struct ShinyDialogBoxState *state = ShinyDialogGetState(hwnd);
+    state->result = ret;
+    state->ended = true;
+}