From d7566f5f5af8778cf1ed647be3897e3a43213375 Mon Sep 17 00:00:00 2001 From: Ondrej Holy Date: Fri, 22 Jan 2021 09:44:55 +0100 Subject: [PATCH] client: Fix exit codes for /help and similar option (#6741) * 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 * Refactored freerdp_client_settings_command_line_status_print_ex Now returns 0 if help or version information was requested. * Do not eliminate original error status. Co-authored-by: akallabeth --- client/Sample/tf_freerdp.c | 6 ++---- client/Wayland/wlfreerdp.c | 10 ++++------ client/Windows/cli/wfreerdp.c | 2 +- client/X11/cli/xfreerdp.c | 19 ++++++++++--------- client/common/cmdline.c | 13 ++++++++----- winpr/include/winpr/cmdline.h | 1 + 6 files changed, 26 insertions(+), 25 deletions(-) diff --git a/client/Sample/tf_freerdp.c b/client/Sample/tf_freerdp.c index 3ba82c783..e9b9fe839 100644 --- a/client/Sample/tf_freerdp.c +++ b/client/Sample/tf_freerdp.c @@ -338,12 +338,10 @@ 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; + 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 329d12009..4a5830689 100644 --- a/client/Wayland/wlfreerdp.c +++ b/client/Wayland/wlfreerdp.c @@ -628,18 +628,16 @@ 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; + + rc = 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; + goto fail; } if (freerdp_client_start(context) != 0) diff --git a/client/Windows/cli/wfreerdp.c b/client/Windows/cli/wfreerdp.c index 7a76eeb9b..e325f8477 100644 --- a/client/Windows/cli/wfreerdp.c +++ b/client/Windows/cli/wfreerdp.c @@ -107,7 +107,7 @@ INT WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine if (status) { - freerdp_client_settings_command_line_status_print(settings, status, argc, argv); + 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 c8a77f335..5b7021944 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,31 @@ 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; + + rc = 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; + 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/client/common/cmdline.c b/client/common/cmdline.c index ed467afb7..66d3c4fff 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,18 @@ 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; + 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) diff --git a/winpr/include/winpr/cmdline.h b/winpr/include/winpr/cmdline.h index 865ee8f25..9276cda8e 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 */