From 3ee4cabcfad3a9ccc3c59be21245b57c17e7ae75 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Thu, 21 Jan 2021 14:21:09 +0100 Subject: [PATCH 1/3] client: Fix exit codes for /help and similar option Currently, non-zero exit code is returned for /version, /buildconfig, /help, /monitor-list, /kbd-list and /kbd-lang-list command-line options for several clients. This is against conventions because 0 is usually returned in such cases. Also, there is potentially another problem that the returned codes overflow on UNIX systems (where the exit code is a number between 0 and 255). Let's fix the clients to return 0 in the mentioned cases to honor conventions and 1 for the command-line parsing errors (or -1 for clients who already use that value). Fixes: https://github.com/FreeRDP/FreeRDP/issues/6686 --- client/Sample/tf_freerdp.c | 9 +++++---- client/Wayland/wlfreerdp.c | 13 +++++++------ client/Windows/cli/wfreerdp.c | 4 ++++ client/X11/cli/xfreerdp.c | 22 +++++++++++++--------- winpr/include/winpr/cmdline.h | 1 + 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/client/Sample/tf_freerdp.c b/client/Sample/tf_freerdp.c index 3ba82c78338..49412cb417c 100644 --- a/client/Sample/tf_freerdp.c +++ b/client/Sample/tf_freerdp.c @@ -338,12 +338,13 @@ int main(int argc, char* argv[]) goto fail; status = freerdp_client_settings_parse_command_line(context->settings, argc, argv, FALSE); - status = - freerdp_client_settings_command_line_status_print(context->settings, status, argc, argv); - if (status) { - rc = 0; + freerdp_client_settings_command_line_status_print(context->settings, status, argc, argv); + + if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) + rc = 0; + goto fail; } diff --git a/client/Wayland/wlfreerdp.c b/client/Wayland/wlfreerdp.c index 329d1200941..d77b47986ea 100644 --- a/client/Wayland/wlfreerdp.c +++ b/client/Wayland/wlfreerdp.c @@ -628,18 +628,19 @@ int main(int argc, char* argv[]) settings = context->settings; status = freerdp_client_settings_parse_command_line(settings, argc, argv, FALSE); - status = freerdp_client_settings_command_line_status_print(settings, status, argc, argv); - if (status) { BOOL list = settings->ListMonitors; + + freerdp_client_settings_command_line_status_print(settings, status, argc, argv); + if (list) wlf_list_monitors(wlc); - freerdp_client_context_free(context); - if (list) - return 0; - return status; + if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) + rc = 0; + + goto fail; } if (freerdp_client_start(context) != 0) diff --git a/client/Windows/cli/wfreerdp.c b/client/Windows/cli/wfreerdp.c index 7a76eeb9b59..b623067e98e 100644 --- a/client/Windows/cli/wfreerdp.c +++ b/client/Windows/cli/wfreerdp.c @@ -108,6 +108,10 @@ INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine if (status) { freerdp_client_settings_command_line_status_print(settings, status, argc, argv); + + if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) + rc = 0; + goto out; } diff --git a/client/X11/cli/xfreerdp.c b/client/X11/cli/xfreerdp.c index c8a77f335f4..a3505b24d3c 100644 --- a/client/X11/cli/xfreerdp.c +++ b/client/X11/cli/xfreerdp.c @@ -34,6 +34,7 @@ int main(int argc, char* argv[]) { + int rc = 1; int status; HANDLE thread; xfContext* xfc; @@ -56,31 +57,34 @@ int main(int argc, char* argv[]) xfc = (xfContext*)context; status = freerdp_client_settings_parse_command_line(context->settings, argc, argv, FALSE); - - status = freerdp_client_settings_command_line_status_print(settings, status, argc, argv); - if (status) { BOOL list = settings->ListMonitors; + + freerdp_client_settings_command_line_status_print(settings, status, argc, argv); + if (list) xf_list_monitors(xfc); - freerdp_client_context_free(context); - if (list) - return 0; - return status; + if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) + rc = 0; + + goto out; } - freerdp_client_start(context); + if (freerdp_client_start(context) != 0) + goto out; thread = freerdp_client_get_thread(context); WaitForSingleObject(thread, INFINITE); GetExitCodeThread(thread, &dwExitCode); + rc = xf_exit_code_from_disconnect_reason(dwExitCode); freerdp_client_stop(context); +out: freerdp_client_context_free(context); - return xf_exit_code_from_disconnect_reason(dwExitCode); + return rc; } diff --git a/winpr/include/winpr/cmdline.h b/winpr/include/winpr/cmdline.h index 865ee8f25c7..9276cda8eb1 100644 --- a/winpr/include/winpr/cmdline.h +++ b/winpr/include/winpr/cmdline.h @@ -81,6 +81,7 @@ #define COMMAND_LINE_STATUS_PRINT_HELP -2002 #define COMMAND_LINE_STATUS_PRINT_VERSION -2003 #define COMMAND_LINE_STATUS_PRINT_BUILDCONFIG -2004 +#define COMMAND_LINE_STATUS_PRINT_LAST -2999 /* Command-Line Macros */ From 531dd81836f2c97fcfcfeabdb9671fb76409ce8d Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 22 Jan 2021 08:40:03 +0100 Subject: [PATCH 2/3] Refactored freerdp_client_settings_command_line_status_print_ex Now returns 0 if help or version information was requested. --- client/Sample/tf_freerdp.c | 7 ++----- client/Wayland/wlfreerdp.c | 5 +---- client/Windows/cli/wfreerdp.c | 6 +----- client/X11/cli/xfreerdp.c | 5 +---- client/common/cmdline.c | 14 +++++++++----- 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/client/Sample/tf_freerdp.c b/client/Sample/tf_freerdp.c index 49412cb417c..e9b9fe8397e 100644 --- a/client/Sample/tf_freerdp.c +++ b/client/Sample/tf_freerdp.c @@ -340,11 +340,8 @@ int main(int argc, char* argv[]) status = freerdp_client_settings_parse_command_line(context->settings, argc, argv, FALSE); if (status) { - freerdp_client_settings_command_line_status_print(context->settings, status, argc, argv); - - if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) - rc = 0; - + rc = freerdp_client_settings_command_line_status_print(context->settings, status, argc, + argv); goto fail; } diff --git a/client/Wayland/wlfreerdp.c b/client/Wayland/wlfreerdp.c index d77b47986ea..4a583068968 100644 --- a/client/Wayland/wlfreerdp.c +++ b/client/Wayland/wlfreerdp.c @@ -632,14 +632,11 @@ int main(int argc, char* argv[]) { BOOL list = settings->ListMonitors; - freerdp_client_settings_command_line_status_print(settings, status, argc, argv); + rc = freerdp_client_settings_command_line_status_print(settings, status, argc, argv); if (list) wlf_list_monitors(wlc); - if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) - rc = 0; - goto fail; } diff --git a/client/Windows/cli/wfreerdp.c b/client/Windows/cli/wfreerdp.c index b623067e98e..e325f84771f 100644 --- a/client/Windows/cli/wfreerdp.c +++ b/client/Windows/cli/wfreerdp.c @@ -107,11 +107,7 @@ INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine if (status) { - freerdp_client_settings_command_line_status_print(settings, status, argc, argv); - - if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) - rc = 0; - + ret = freerdp_client_settings_command_line_status_print(settings, status, argc, argv); goto out; } diff --git a/client/X11/cli/xfreerdp.c b/client/X11/cli/xfreerdp.c index a3505b24d3c..5b702194448 100644 --- a/client/X11/cli/xfreerdp.c +++ b/client/X11/cli/xfreerdp.c @@ -61,14 +61,11 @@ int main(int argc, char* argv[]) { BOOL list = settings->ListMonitors; - freerdp_client_settings_command_line_status_print(settings, status, argc, argv); + rc = freerdp_client_settings_command_line_status_print(settings, status, argc, argv); if (list) xf_list_monitors(xfc); - if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) - rc = 0; - goto out; } diff --git a/client/common/cmdline.c b/client/common/cmdline.c index ed467afb7d0..dc8367b7cd2 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -1403,14 +1403,14 @@ int freerdp_client_settings_command_line_status_print_ex(rdpSettings* settings, if (status == COMMAND_LINE_STATUS_PRINT_VERSION) { freerdp_client_print_version(); - return COMMAND_LINE_STATUS_PRINT_VERSION; + goto out; } if (status == COMMAND_LINE_STATUS_PRINT_BUILDCONFIG) { freerdp_client_print_version(); freerdp_client_print_buildconfig(); - return COMMAND_LINE_STATUS_PRINT_BUILDCONFIG; + goto out; } else if (status == COMMAND_LINE_STATUS_PRINT) { @@ -1465,15 +1465,19 @@ int freerdp_client_settings_command_line_status_print_ex(rdpSettings* settings, settings->ListMonitors = TRUE; } - return COMMAND_LINE_STATUS_PRINT; + goto out; } else if (status < 0) { freerdp_client_print_command_line_help_ex(argc, argv, custom); - return COMMAND_LINE_STATUS_PRINT_HELP; + status = COMMAND_LINE_STATUS_PRINT_HELP; + goto out; } - return 0; +out: + if (status <= COMMAND_LINE_STATUS_PRINT && status >= COMMAND_LINE_STATUS_PRINT_LAST) + return 0; + return status; } static BOOL ends_with(const char* str, const char* ext) From 050a68fec901030c7428852f8f536ace055eb2f7 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 22 Jan 2021 09:32:51 +0100 Subject: [PATCH 3/3] Do not eliminate original error status. --- client/common/cmdline.c | 1 - 1 file changed, 1 deletion(-) diff --git a/client/common/cmdline.c b/client/common/cmdline.c index dc8367b7cd2..66d3c4fffef 100644 --- a/client/common/cmdline.c +++ b/client/common/cmdline.c @@ -1470,7 +1470,6 @@ int freerdp_client_settings_command_line_status_print_ex(rdpSettings* settings, else if (status < 0) { freerdp_client_print_command_line_help_ex(argc, argv, custom); - status = COMMAND_LINE_STATUS_PRINT_HELP; goto out; }