Remove MAX_CONFIGS limit using WM_MENUCOMMAND for menu messages

We currently use WM_COMMAND message which is delivered with the
ID of the menu item requiring a unique ID for every command
(connect, disconnect etc..) for each connection profile. Instead,
use WM_MENUCOMMAND so that the message delivers a handle to the
menu and the position index of the menu item.

Connection menu array is now dynamically allocated. Yet, there
is still a limitation on the number of configs as the config
index + mgmt_port_offset must be < 65536 to be usable as a port
number. The error message shown for "too many configs" is reworded.
(English language file only).

Note: The current way of selecting the management port based on the
index of the config file increases chances of port conflicts
when the number of configs is large. It could be useful to change
this logic but that is beyond the cope of this PR.

Signed-off-by: Selva Nair <selva.nair@gmail.com>
pull/396/merge
Selva Nair 2021-02-27 14:40:15 -05:00
parent 6b1372d886
commit c47c3bf81a
6 changed files with 122 additions and 70 deletions

82
main.c
View File

@ -488,6 +488,8 @@ HandleCopyDataMessage(const COPYDATASTRUCT *copy_data)
LRESULT CALLBACK WindowProcedure (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) LRESULT CALLBACK WindowProcedure (HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam)
{ {
static UINT s_uTaskbarRestart; static UINT s_uTaskbarRestart;
int conn_id = 0;
MENUINFO minfo = {.cbSize = sizeof(MENUINFO)};
switch (message) { switch (message) {
case WM_CREATE: case WM_CREATE:
@ -532,49 +534,65 @@ LRESULT CALLBACK WindowProcedure (HWND hwnd, UINT message, WPARAM wParam, LPARAM
HandleCopyDataMessage((COPYDATASTRUCT*) lParam); HandleCopyDataMessage((COPYDATASTRUCT*) lParam);
return TRUE; /* lets the sender free copy_data */ return TRUE; /* lets the sender free copy_data */
case WM_COMMAND: case WM_MENUCOMMAND:
if ( (LOWORD(wParam) >= IDM_CONNECTMENU) && (LOWORD(wParam) < IDM_CONNECTMENU + MAX_CONFIGS) ) { /* Get the menu item id and save it in wParam for use below */
StartOpenVPN(&o.conn[LOWORD(wParam) - IDM_CONNECTMENU]); wParam = GetMenuItemID((HMENU) lParam, wParam);
}
if ( (LOWORD(wParam) >= IDM_DISCONNECTMENU) && (LOWORD(wParam) < IDM_DISCONNECTMENU + MAX_CONFIGS) ) { /* we first check global menu items which do not require a connnection index */
StopOpenVPN(&o.conn[LOWORD(wParam) - IDM_DISCONNECTMENU]);
}
if ( (LOWORD(wParam) >= IDM_RECONNECTMENU) && (LOWORD(wParam) < IDM_RECONNECTMENU + MAX_CONFIGS) ) {
RestartOpenVPN(&o.conn[LOWORD(wParam) - IDM_RECONNECTMENU]);
}
if ( (LOWORD(wParam) >= IDM_STATUSMENU) && (LOWORD(wParam) < IDM_STATUSMENU + MAX_CONFIGS) ) {
ShowWindow(o.conn[LOWORD(wParam) - IDM_STATUSMENU].hwndStatus, SW_SHOW);
}
if ( (LOWORD(wParam) >= IDM_VIEWLOGMENU) && (LOWORD(wParam) < IDM_VIEWLOGMENU + MAX_CONFIGS) ) {
ViewLog(LOWORD(wParam) - IDM_VIEWLOGMENU);
}
if ( (LOWORD(wParam) >= IDM_EDITMENU) && (LOWORD(wParam) < IDM_EDITMENU + MAX_CONFIGS) ) {
EditConfig(LOWORD(wParam) - IDM_EDITMENU);
}
if ( (LOWORD(wParam) >= IDM_CLEARPASSMENU) && (LOWORD(wParam) < IDM_CLEARPASSMENU + MAX_CONFIGS) ) {
ResetSavePasswords(&o.conn[LOWORD(wParam) - IDM_CLEARPASSMENU]);
}
#ifndef DISABLE_CHANGE_PASSWORD
if ( (LOWORD(wParam) >= IDM_PASSPHRASEMENU) && (LOWORD(wParam) < IDM_PASSPHRASEMENU + MAX_CONFIGS) ) {
ShowChangePassphraseDialog(&o.conn[LOWORD(wParam) - IDM_PASSPHRASEMENU]);
}
#endif
if (LOWORD(wParam) == IDM_IMPORT) { if (LOWORD(wParam) == IDM_IMPORT) {
ImportConfigFile(); ImportConfigFile();
} }
if (LOWORD(wParam) == IDM_SETTINGS) { else if (LOWORD(wParam) == IDM_SETTINGS) {
ShowSettingsDialog(); ShowSettingsDialog();
} }
if (LOWORD(wParam) == IDM_CLOSE) { else if (LOWORD(wParam) == IDM_CLOSE) {
CloseApplication(hwnd); CloseApplication(hwnd);
} }
if (LOWORD(wParam) == IDM_SERVICE_START) { else if (LOWORD(wParam) == IDM_SERVICE_START) {
MyStartService(); MyStartService();
} }
if (LOWORD(wParam) == IDM_SERVICE_STOP) { else if (LOWORD(wParam) == IDM_SERVICE_STOP) {
MyStopService(); MyStopService();
} }
if (LOWORD(wParam) == IDM_SERVICE_RESTART) MyReStartService(); else if (LOWORD(wParam) == IDM_SERVICE_RESTART) {
MyReStartService();
}
/* rest of the handlers require a connection id */
else {
minfo.fMask = MIM_MENUDATA;
GetMenuInfo((HMENU) lParam, &minfo);
conn_id = (INT) minfo.dwMenuData;
if (conn_id < 0 || conn_id >= o.num_configs) break; /* ignore invalid connection id */
}
/* reach here only if the command did not match any global items and a valid connection id is available */
if (LOWORD(wParam) == IDM_CONNECTMENU) {
StartOpenVPN(&o.conn[conn_id]);
}
else if (LOWORD(wParam) == IDM_DISCONNECTMENU) {
StopOpenVPN(&o.conn[conn_id]);
}
else if (LOWORD(wParam) == IDM_RECONNECTMENU) {
RestartOpenVPN(&o.conn[conn_id]);
}
else if (LOWORD(wParam) == IDM_STATUSMENU) {
ShowWindow(o.conn[conn_id].hwndStatus, SW_SHOW);
}
else if (LOWORD(wParam) == IDM_VIEWLOGMENU) {
ViewLog(conn_id);
}
else if (LOWORD(wParam) == IDM_EDITMENU) {
EditConfig(conn_id);
}
else if (LOWORD(wParam) == IDM_CLEARPASSMENU) {
ResetSavePasswords(&o.conn[conn_id]);
}
#ifndef DISABLE_CHANGE_PASSWORD
else if (LOWORD(wParam) == IDM_PASSPHRASEMENU) {
ShowChangePassphraseDialog(&o.conn[conn_id]);
}
#endif
break; break;
case WM_CLOSE: case WM_CLOSE:

View File

@ -403,6 +403,7 @@ BuildFileList()
int recurse_depth = 20; /* maximum number of levels below config_dir to recurse into */ int recurse_depth = 20; /* maximum number of levels below config_dir to recurse into */
int flags = 0; int flags = 0;
int root = 0; int root = 0;
int max_configs = (1<<16) - o.mgmt_port_offset;
if (o.silent_connection) if (o.silent_connection)
issue_warnings = false; issue_warnings = false;
@ -437,12 +438,12 @@ BuildFileList()
if (o.num_configs == 0 && issue_warnings) if (o.num_configs == 0 && issue_warnings)
ShowLocalizedMsg(IDS_NFO_NO_CONFIGS, o.config_dir, o.global_config_dir); ShowLocalizedMsg(IDS_NFO_NO_CONFIGS, o.config_dir, o.global_config_dir);
/* More than MAX_CONFIGS are ignored in the menu listing */ /* More than max_configs are ignored in the menu listing */
if (o.num_configs > MAX_CONFIGS) if (o.num_configs > max_configs)
{ {
if (issue_warnings) if (issue_warnings)
ShowLocalizedMsg(IDS_ERR_MANY_CONFIGS, o.num_configs); ShowLocalizedMsg(IDS_ERR_MANY_CONFIGS, max_configs);
o.num_configs = MAX_CONFIGS; /* menus don't work with more -- ignore the rest */ o.num_configs = max_configs; /* management-port cant handle more -- ignore the rest */
} }
/* if adding groups, activate non-empty ones */ /* if adding groups, activate non-empty ones */

View File

@ -42,8 +42,6 @@ typedef struct connection connection_t;
* including the option name itself. * including the option name itself.
*/ */
#define MAX_PARMS 5 /* Max number of parameters per option */ #define MAX_PARMS 5 /* Max number of parameters per option */
/* Menu ids are constructed as 12 bits for config number, 4 bits for action */
#define MAX_CONFIGS (1<<12)
typedef enum { typedef enum {
service_noaccess = -1, service_noaccess = -1,

View File

@ -293,7 +293,7 @@ BEGIN
IDS_ERR_START_CONF_EDITOR "Error starting config-editor: %s" IDS_ERR_START_CONF_EDITOR "Error starting config-editor: %s"
/* OpenVPN */ /* OpenVPN */
IDS_ERR_MANY_CONFIGS "OpenVPN GUI does not support more than %d configs. Please contact the author if you have the need for more." IDS_ERR_MANY_CONFIGS "For the given parameter set, OpenVPN GUI can not support more than %d configs. Configs beyond this number are ignored."
IDS_NFO_NO_CONFIGS "No readable connection profiles (config files) found.\n"\ IDS_NFO_NO_CONFIGS "No readable connection profiles (config files) found.\n"\
"Use the ""Import File.."" menu or copy your config files to ""%s"" or ""%s""." "Use the ""Import File.."" menu or copy your config files to ""%s"" or ""%s""."
IDS_ERR_CONFIG_NOT_AUTHORIZED "Starting this connection (%s) requires membership in "\ IDS_ERR_CONFIG_NOT_AUTHORIZED "Starting this connection (%s) requires membership in "\

81
tray.c
View File

@ -42,7 +42,8 @@
/* Popup Menus */ /* Popup Menus */
HMENU hMenu; HMENU hMenu;
HMENU hMenuConn[MAX_CONFIGS]; HMENU *hMenuConn;
int hmenu_size = 0; /* allocated size of hMenuConn array */
HMENU hMenuService; HMENU hMenuService;
HBITMAP hbmpConnecting; HBITMAP hbmpConnecting;
@ -71,6 +72,27 @@ CreateBitmaps()
} }
} }
/*
* Reallocate hMenuConn as required. In case of not enough memory,
* o.num_config is reset to max space available value so that the
* program can continue.
*/
void
AllocateConnectionMenu()
{
if (hmenu_size >= o.num_configs) return;
HMENU *tmp = (HMENU *) realloc(hMenuConn, sizeof(HMENU)*(o.num_configs + 50));
if (tmp) {
hmenu_size = o.num_configs + 50;
hMenuConn = tmp;
}
else {
o.num_configs = hmenu_size;
MsgToEventLog(EVENTLOG_ERROR_TYPE, L"Allocation of hMenuConn failed. Ignoring configs beyond index = %d", o.num_configs);
}
return;
}
/* Create popup menus */ /* Create popup menus */
void void
CreatePopupMenus() CreatePopupMenus()
@ -81,11 +103,18 @@ CreatePopupMenus()
*/ */
assert(o.num_groups > 0); assert(o.num_groups > 0);
AllocateConnectionMenu();
CreateBitmaps(); CreateBitmaps();
MENUINFO minfo = {.cbSize = sizeof(MENUINFO)};
for (int i = 0; i < o.num_configs; i++) for (int i = 0; i < o.num_configs; i++)
{ {
hMenuConn[i] = CreatePopupMenu(); hMenuConn[i] = CreatePopupMenu();
/* Save the connection index in the menu.*/
minfo.fMask = MIM_MENUDATA;
minfo.dwMenuData = i;
SetMenuInfo(hMenuConn[i], &minfo);
} }
for (int i = 0; i < o.num_groups; i++) for (int i = 0; i < o.num_groups; i++)
{ {
@ -98,6 +127,12 @@ CreatePopupMenus()
hMenuService = CreatePopupMenu(); hMenuService = CreatePopupMenu();
hMenu = o.groups[0].menu; /* the first group menu is also the root menu */ hMenu = o.groups[0].menu; /* the first group menu is also the root menu */
/* Set notify by position style for the top menu - gets automatically applied to sub-menus */
minfo.fMask = MIM_STYLE;
GetMenuInfo(hMenu, &minfo);
minfo.dwStyle |= MNS_NOTIFYBYPOS;
SetMenuInfo(hMenu, &minfo);
if (o.num_configs == 1) { if (o.num_configs == 1) {
/* Create Main menu with actions */ /* Create Main menu with actions */
if (o.service_only == 0) { if (o.service_only == 0) {
@ -190,21 +225,21 @@ CreatePopupMenus()
/* Create popup menus for every connection */ /* Create popup menus for every connection */
for (int i = 0; i < o.num_configs; i++) { for (int i = 0; i < o.num_configs; i++) {
if (o.service_only == 0) { if (o.service_only == 0) {
AppendMenu(hMenuConn[i], MF_STRING, IDM_CONNECTMENU + i, LoadLocalizedString(IDS_MENU_CONNECT)); AppendMenu(hMenuConn[i], MF_STRING, IDM_CONNECTMENU, LoadLocalizedString(IDS_MENU_CONNECT));
AppendMenu(hMenuConn[i], MF_STRING, IDM_DISCONNECTMENU + i, LoadLocalizedString(IDS_MENU_DISCONNECT)); AppendMenu(hMenuConn[i], MF_STRING, IDM_DISCONNECTMENU, LoadLocalizedString(IDS_MENU_DISCONNECT));
AppendMenu(hMenuConn[i], MF_STRING, IDM_RECONNECTMENU + i, LoadLocalizedString(IDS_MENU_RECONNECT)); AppendMenu(hMenuConn[i], MF_STRING, IDM_RECONNECTMENU, LoadLocalizedString(IDS_MENU_RECONNECT));
AppendMenu(hMenuConn[i], MF_STRING, IDM_STATUSMENU + i, LoadLocalizedString(IDS_MENU_STATUS)); AppendMenu(hMenuConn[i], MF_STRING, IDM_STATUSMENU, LoadLocalizedString(IDS_MENU_STATUS));
AppendMenu(hMenuConn[i], MF_SEPARATOR, 0, 0); AppendMenu(hMenuConn[i], MF_SEPARATOR, 0, 0);
} }
AppendMenu(hMenuConn[i], MF_STRING, IDM_VIEWLOGMENU + i, LoadLocalizedString(IDS_MENU_VIEWLOG)); AppendMenu(hMenuConn[i], MF_STRING, IDM_VIEWLOGMENU, LoadLocalizedString(IDS_MENU_VIEWLOG));
AppendMenu(hMenuConn[i], MF_STRING, IDM_EDITMENU + i, LoadLocalizedString(IDS_MENU_EDITCONFIG)); AppendMenu(hMenuConn[i], MF_STRING, IDM_EDITMENU, LoadLocalizedString(IDS_MENU_EDITCONFIG));
AppendMenu(hMenuConn[i], MF_STRING, IDM_CLEARPASSMENU + i, LoadLocalizedString(IDS_MENU_CLEARPASS)); AppendMenu(hMenuConn[i], MF_STRING, IDM_CLEARPASSMENU, LoadLocalizedString(IDS_MENU_CLEARPASS));
#ifndef DISABLE_CHANGE_PASSWORD #ifndef DISABLE_CHANGE_PASSWORD
if (o.conn[i].flags & FLAG_ALLOW_CHANGE_PASSPHRASE) if (o.conn[i].flags & FLAG_ALLOW_CHANGE_PASSPHRASE)
AppendMenu(hMenuConn[i], MF_STRING, IDM_PASSPHRASEMENU + i, LoadLocalizedString(IDS_MENU_PASSPHRASE)); AppendMenu(hMenuConn[i], MF_STRING, IDM_PASSPHRASEMENU, LoadLocalizedString(IDS_MENU_PASSPHRASE));
#endif #endif
SetMenuStatusById(i, o.conn[i].state); SetMenuStatusById(i, o.conn[i].state);
@ -515,29 +550,29 @@ SetMenuStatusById(int i, conn_state_t state)
if (state == disconnected) if (state == disconnected)
{ {
EnableMenuItem(hMenuConn[i], IDM_CONNECTMENU + i, MF_ENABLED); EnableMenuItem(hMenuConn[i], IDM_CONNECTMENU, MF_ENABLED);
EnableMenuItem(hMenuConn[i], IDM_DISCONNECTMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_DISCONNECTMENU, MF_GRAYED);
EnableMenuItem(hMenuConn[i], IDM_RECONNECTMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_RECONNECTMENU, MF_GRAYED);
EnableMenuItem(hMenuConn[i], IDM_STATUSMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_STATUSMENU, MF_GRAYED);
} }
else if (state == connecting || state == resuming || state == connected) else if (state == connecting || state == resuming || state == connected)
{ {
EnableMenuItem(hMenuConn[i], IDM_CONNECTMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_CONNECTMENU, MF_GRAYED);
EnableMenuItem(hMenuConn[i], IDM_DISCONNECTMENU + i, MF_ENABLED); EnableMenuItem(hMenuConn[i], IDM_DISCONNECTMENU, MF_ENABLED);
EnableMenuItem(hMenuConn[i], IDM_RECONNECTMENU + i, MF_ENABLED); EnableMenuItem(hMenuConn[i], IDM_RECONNECTMENU, MF_ENABLED);
EnableMenuItem(hMenuConn[i], IDM_STATUSMENU + i, MF_ENABLED); EnableMenuItem(hMenuConn[i], IDM_STATUSMENU, MF_ENABLED);
} }
else if (state == disconnecting) else if (state == disconnecting)
{ {
EnableMenuItem(hMenuConn[i], IDM_CONNECTMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_CONNECTMENU, MF_GRAYED);
EnableMenuItem(hMenuConn[i], IDM_DISCONNECTMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_DISCONNECTMENU, MF_GRAYED);
EnableMenuItem(hMenuConn[i], IDM_RECONNECTMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_RECONNECTMENU, MF_GRAYED);
EnableMenuItem(hMenuConn[i], IDM_STATUSMENU + i, MF_ENABLED); EnableMenuItem(hMenuConn[i], IDM_STATUSMENU, MF_ENABLED);
} }
if (c->flags & (FLAG_SAVE_AUTH_PASS | FLAG_SAVE_KEY_PASS)) if (c->flags & (FLAG_SAVE_AUTH_PASS | FLAG_SAVE_KEY_PASS))
EnableMenuItem(hMenuConn[i], IDM_CLEARPASSMENU + i, MF_ENABLED); EnableMenuItem(hMenuConn[i], IDM_CLEARPASSMENU, MF_ENABLED);
else else
EnableMenuItem(hMenuConn[i], IDM_CLEARPASSMENU + i, MF_GRAYED); EnableMenuItem(hMenuConn[i], IDM_CLEARPASSMENU, MF_GRAYED);
} }
} }

14
tray.h
View File

@ -34,13 +34,13 @@
#define IDM_IMPORT 224 #define IDM_IMPORT 224
#define IDM_CONNECTMENU 300 #define IDM_CONNECTMENU 300
#define IDM_DISCONNECTMENU (MAX_CONFIGS + IDM_CONNECTMENU) #define IDM_DISCONNECTMENU (1 + IDM_CONNECTMENU)
#define IDM_STATUSMENU (MAX_CONFIGS + IDM_DISCONNECTMENU) #define IDM_STATUSMENU (1 + IDM_DISCONNECTMENU)
#define IDM_VIEWLOGMENU (MAX_CONFIGS + IDM_STATUSMENU) #define IDM_VIEWLOGMENU (1 + IDM_STATUSMENU)
#define IDM_EDITMENU (MAX_CONFIGS + IDM_VIEWLOGMENU) #define IDM_EDITMENU (1 + IDM_VIEWLOGMENU)
#define IDM_PASSPHRASEMENU (MAX_CONFIGS + IDM_EDITMENU) #define IDM_PASSPHRASEMENU (1 + IDM_EDITMENU)
#define IDM_CLEARPASSMENU (MAX_CONFIGS + IDM_PASSPHRASEMENU) #define IDM_CLEARPASSMENU (1 + IDM_PASSPHRASEMENU)
#define IDM_RECONNECTMENU (MAX_CONFIGS + IDM_CLEARPASSMENU) #define IDM_RECONNECTMENU (1 + IDM_CLEARPASSMENU)
void CreatePopupMenus(); void CreatePopupMenus();
void OnNotifyTray(LPARAM); void OnNotifyTray(LPARAM);