> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> ---
> src/spice-session.c | 31 +++++++++++++++++++++++--------
> tests/session.c | 20 ++++++++++----------
> 2 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/src/spice-session.c b/src/spice-session.c
> index d2aa5e7..6faf0f2 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -400,17 +400,32 @@ static gchar* spice_uri_create(SpiceSession *session)
> if (s->unix_path != NULL) {
> return g_strdup_printf(URI_SCHEME_SPICE_UNIX "%s", s->unix_path);
> } else if (s->host != NULL) {
> + GString *str;
> g_return_val_if_fail(s->port != NULL || s->tls_port != NULL, NULL);
>
> - GString *str = g_string_new(URI_SCHEME_SPICE);
> + if (!!s->tls_port + !!s->port == 1) {
> + /* use spice://foo:4390 or spice+tls://.. form */
> + const char *port;
>
> - g_string_append(str, s->host);
> - g_string_append(str, "?");
> - if (s->port != NULL) {
> - g_string_append_printf(str, "port=%s&", s->port);
> - }
> - if (s->tls_port != NULL) {
> - g_string_append_printf(str, "tls-port=%s", s->tls_port);
> + if (s->tls_port) {
> + str = g_string_new(URI_SCHEME_SPICE_TLS);
> + port = s->tls_port;
> + } else {
> + str = g_string_new(URI_SCHEME_SPICE);
> + port = s->port;
> + }
> + g_string_append_printf(str, "%s:%s", s->host, port);
> + } else {
> + /* use spice://foo?port=4390&tls-port= form */
> + str = g_string_new(URI_SCHEME_SPICE);
> + g_string_append(str, s->host);
> + g_string_append(str, "?");
> + if (s->port != NULL) {
> + g_string_append_printf(str, "port=%s&", s->port);
> + }
> + if (s->tls_port != NULL) {
> + g_string_append_printf(str, "tls-port=%s", s->tls_port);
> + }
> }
> return g_string_free(str, FALSE);
> }
Looks more complicated than should be, specially the !!s->tls_port + !!s->port == 1
check. Considered that the row above remove the case where none are set
the check can just be if both are set and the reverse, I propose (tested) a
if (s->unix_path != NULL) {
return g_strdup_printf(URI_SCHEME_SPICE_UNIX "%s", s->unix_path);
} else if (s->host != NULL) {
const char *port, *scheme;
g_return_val_if_fail(s->port != NULL || s->tls_port != NULL, NULL);
if (s->tls_port && s->port) {
/* both set, use spice://foo?port=4390&tls-port= form */
return g_strdup_printf(URI_SCHEME_SPICE "%s?port=%s&tls-port=%s",
s->host, s->port, s->tls_port);
}
/* one set, use spice://foo:4390 or spice+tls://.. form */
if (s->tls_port) {
scheme = URI_SCHEME_SPICE_TLS;
port = s->tls_port;
} else {
scheme = URI_SCHEME_SPICE;
port = s->port;
}
return g_strdup_printf("%s%s:%s", scheme, s->host, port);
}
> diff --git a/tests/session.c b/tests/session.c
> index 413d812..fc874fc 100644
> --- a/tests/session.c
> +++ b/tests/session.c
> @@ -201,28 +201,28 @@ static void test_session_uri_ipv4_good(void)
> "localhost",
> NULL, NULL,
> "spice://localhost?port=5900&tls-port=",
> - "spice://localhost?port=5900&" },
> + "spice://localhost:5900" },
> { "5910", NULL,
> "localhost",
> "user", NULL,
> "spice://user@localhost?tls-port=&port=5910",
> - "spice://localhost?port=5910&" },
> + "spice://localhost:5910" },
> { NULL, "5920",
> "localhost",
> "user", "password",
> "spice://user@localhost?tls-port=5920&port=&password=password",
> - "spice://localhost?tls-port=5920",
> + "spice+tls://localhost:5920",
> "password may be visible in process listings"},
> { NULL, "5930",
> "localhost",
> NULL, NULL,
> "spice://localhost?port=&tls-port=5930",
> - "spice://localhost?tls-port=5930" },
> + "spice+tls://localhost:5930" },
> { "42", NULL,
> "localhost",
> NULL, NULL,
> "spice://localhost:42",
> - "spice://localhost?port=42&" },
> + "spice://localhost:42" },
> { "42", "5930",
> "localhost",
> NULL, NULL,
> @@ -246,28 +246,28 @@ static void test_session_uri_ipv6_good(void)
> "[2010:836B:4179::836B:4179]",
> NULL, NULL,
> "spice://[2010:836B:4179::836B:4179]?port=5900&tls-port=",
> - "spice://[2010:836B:4179::836B:4179]?port=5900&" },
> + "spice://[2010:836B:4179::836B:4179]:5900" },
> { "5910", NULL,
> "[::192.9.5.5]",
> "user", NULL,
> "spice://user@[::192.9.5.5]?tls-port=&port=5910",
> - "spice://[::192.9.5.5]?port=5910&" },
> + "spice://[::192.9.5.5]:5910" },
> { NULL, "5920",
> "[3ffe:2a00:100:7031::1]",
> "user", "password",
> "spice://user@[3ffe:2a00:100:7031::1]?tls-port=5920&port=&password=password",
> - "spice://[3ffe:2a00:100:7031::1]?tls-port=5920",
> + "spice+tls://[3ffe:2a00:100:7031::1]:5920",
> "password may be visible in process listings"},
> { NULL, "5930",
> "[1080:0:0:0:8:800:200C:417A]",
> NULL, NULL,
> "spice://[1080:0:0:0:8:800:200C:417A]?port=&tls-port=5930",
> - "spice://[1080:0:0:0:8:800:200C:417A]?tls-port=5930" },
> + "spice+tls://[1080:0:0:0:8:800:200C:417A]:5930" },
> { "42", NULL,
> "[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]",
> NULL, NULL,
> "spice://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:42",
> - "spice://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]?port=42&" },
> + "spice://[FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]:42" },
> { "42", "5930",
> "[::192.9.5.5]",
> NULL, NULL,
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel