From 3627dc6168360f72979ebc905c929ea3a53648a4 Mon Sep 17 00:00:00 2001 From: Selva Nair Date: Mon, 23 Jan 2023 13:53:28 -0500 Subject: [PATCH] fixup! Check return value of SetProp Remove CHECK_NULL_PARAM and other over-zealous checks on the return value of GetProp. In pkcs11.c if SetProp() fails just do not use bold font for header instead of leaking the font resource. Also correct a bad fixup in commit 80697ecae6: hfontProp was not set! (to be squashed with previous commit) Signed-off-by: Selva Nair --- as.c | 4 +--- openvpn.c | 13 ------------- openvpn.h | 7 ------- pkcs11.c | 29 +++++++---------------------- proxy.c | 1 - 5 files changed, 8 insertions(+), 46 deletions(-) diff --git a/as.c b/as.c index c46a751..6028688 100644 --- a/as.c +++ b/as.c @@ -252,7 +252,6 @@ CRDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_COMMAND: param = (auth_param_t*)GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(param); switch (LOWORD(wParam)) { case ID_EDT_RESPONSE: @@ -630,8 +629,6 @@ ImportProfileFromURLDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lPa case WM_COMMAND: type = (server_type_t) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(type); - switch (LOWORD(wParam)) { case ID_EDT_AUTH_USER: @@ -702,6 +699,7 @@ ImportProfileFromURLDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lPa return TRUE; case WM_NCDESTROY: + RemoveProp(hwndDlg, cfgProp); break; } diff --git a/openvpn.c b/openvpn.c index 215373f..2ee1427 100644 --- a/openvpn.c +++ b/openvpn.c @@ -586,7 +586,6 @@ UserAuthDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_COMMAND: param = (auth_param_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(param); switch (LOWORD(wParam)) { case ID_EDT_AUTH_USER: @@ -673,7 +672,6 @@ UserAuthDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_CLOSE: EndDialog(hwndDlg, LOWORD(wParam)); param = (auth_param_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(param); StopOpenVPN(param->c); return TRUE; @@ -772,7 +770,6 @@ GenericPassDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_COMMAND: param = (auth_param_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(param); const char *template; char *fmt; switch (LOWORD(wParam)) @@ -862,7 +859,6 @@ GenericPassDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) * other than GET_CONFIG. The state is in lParam. */ param = (auth_param_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(param); if ((param->flags & FLAG_CR_TYPE_CRTEXT) && strcmp((const char *) lParam, "GET_CONFIG")) { @@ -926,7 +922,6 @@ PrivKeyPassDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_COMMAND: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); switch (LOWORD(wParam)) { case ID_CHK_SAVE_PASS: @@ -2041,7 +2036,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_COMMAND: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); switch (LOWORD(wParam)) { case ID_DISCONNECT: @@ -2077,7 +2071,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_CLOSE: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); if (c->state != disconnected && c->state != detached) ShowWindow(hwndDlg, SW_HIDE); else @@ -2094,7 +2087,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_OVPN_RELEASE: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); c->state = reconnecting; SetDlgItemText(c->hwndStatus, ID_TXT_STATUS, LoadLocalizedString(IDS_NFO_STATE_RECONNECTING)); SetDlgItemTextW(c->hwndStatus, ID_TXT_IP, L""); @@ -2104,7 +2096,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_OVPN_STOP: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); /* external messages can trigger when we are not ready -- check the state */ if (!IsWindowEnabled(GetDlgItem(c->hwndStatus, ID_DISCONNECT)) || c->state == onhold) @@ -2125,7 +2116,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_OVPN_DETACH: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); /* just stop the thread keeping openvpn.exe running */ c->state = detaching; EnableWindow(GetDlgItem(c->hwndStatus, ID_DISCONNECT), FALSE); @@ -2135,7 +2125,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_OVPN_SUSPEND: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); c->state = suspending; EnableWindow(GetDlgItem(c->hwndStatus, ID_DISCONNECT), FALSE); EnableWindow(GetDlgItem(c->hwndStatus, ID_RESTART), FALSE); @@ -2148,7 +2137,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_TIMER: PrintDebug(L"WM_TIMER message with wParam = %lu", wParam); c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); if (wParam == IDT_STOP_TIMER) { /* openvpn failed to respond to stop signal -- terminate */ @@ -2160,7 +2148,6 @@ StatusDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_OVPN_RESTART: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); /* external messages can trigger when we are not ready -- check the state */ if (IsWindowEnabled(GetDlgItem(c->hwndStatus, ID_RESTART))) { diff --git a/openvpn.h b/openvpn.h index b2c5b57..acec295 100644 --- a/openvpn.h +++ b/openvpn.h @@ -25,13 +25,6 @@ #include "options.h" -#define CHECK_NULL_PARAM(p) \ - do { if (p) break; \ - MsgToEventLog(EVENTLOG_ERROR_TYPE, L"%hs:%d GetProp returned null", \ - __func__, __LINE__); \ - return false; \ - } while(0) - #define TRY_SETPROP(hwnd, name, p) \ do { if (SetPropW(hwnd, name, p)) break; \ MsgToEventLog(EVENTLOG_ERROR_TYPE, L"%hs:%d GetProp returned null", \ diff --git a/pkcs11.c b/pkcs11.c index 74336de..426fda2 100644 --- a/pkcs11.c +++ b/pkcs11.c @@ -37,7 +37,7 @@ #include extern options_t o; -static const wchar_t *hfontProp; +static const wchar_t *hfontProp = L"header_font"; /* state of list array */ #define STATE_GET_COUNT 1 @@ -414,10 +414,13 @@ pkcs11_listview_init(HWND parent) lf.lfWeight = FW_BOLD; HFONT hfb = CreateFontIndirect(&lf); - if (hfb) + if (hfb && SetPropW(parent, hfontProp, (HANDLE)hfb)) { SendMessage(ListView_GetHeader(lv), WM_SETFONT, (WPARAM)hfb, 1); - SetPropW(parent, hfontProp, (HANDLE)hfb); /* failure here is not critical */ + } + else if (hfb) + { + DeleteObject(hfb); } } @@ -452,13 +455,6 @@ static void CALLBACK pkcs11_listview_fill(HWND hwnd, UINT UNUSED msg, UINT_PTR id, DWORD UNUSED now) { connection_t *c = (connection_t *) GetProp(hwnd, cfgProp); - - if (!c) - { - KillTimer(hwnd, id); - return; - } - struct pkcs11_list *l = &c->pkcs11_list; HWND lv = GetDlgItem(hwnd, ID_LVW_PKCS11); @@ -522,12 +518,6 @@ static void pkcs11_listview_reset(HWND parent) { connection_t *c = (connection_t *) GetProp(parent, cfgProp); - - if (!c) - { - return; - } - struct pkcs11_list *l = &c->pkcs11_list; HWND lv = GetDlgItem(parent, ID_LVW_PKCS11); @@ -599,7 +589,6 @@ QueryPkcs11DialogProc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_COMMAND: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); if (LOWORD(wParam) == IDOK) { HWND lv = GetDlgItem(hwndDlg, ID_LVW_PKCS11); @@ -649,7 +638,6 @@ QueryPkcs11DialogProc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_NOTIFY: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); if (((NMHDR *)lParam)->idFrom == ID_LVW_PKCS11) { NMITEMACTIVATE *ln = (NMITEMACTIVATE *) lParam; @@ -667,10 +655,7 @@ QueryPkcs11DialogProc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case WM_CLOSE: c = (connection_t *) GetProp(hwndDlg, cfgProp); - if (c) - { - StopOpenVPN(c); - } + StopOpenVPN(c); EndDialog(hwndDlg, wParam); return TRUE; diff --git a/proxy.c b/proxy.c index 97492cb..9c0930f 100644 --- a/proxy.c +++ b/proxy.c @@ -361,7 +361,6 @@ ProxyAuthDialogFunc(HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lParam) case IDOK: c = (connection_t *) GetProp(hwndDlg, cfgProp); - CHECK_NULL_PARAM(c); proxy_type = (c->proxy_type == http ? "HTTP" : "SOCKS"); snprintf(fmt, sizeof(fmt), "username \"%s Proxy\" \"%%s\"", proxy_type);