From 3ee4cabcfad3a9ccc3c59be21245b57c17e7ae75 Mon Sep 17 00:00:00 2001

From: Ondrej Holy <oholy@redhat.com>

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 <akallabeth@posteo.net>

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 <akallabeth@posteo.net>

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;

 	}