[Spice-devel] [PATCH spice-gtk v2 3/5] spice-uri: Set user & password to NULL when not specified

[Frediano Ziglio]

> 
> Avoid using old values after parsing a new uri.
> 
> Related: rhbz#1335239
> ---
>  src/spice-uri.c        | 3 +++
>  tests/test-spice-uri.c | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/spice-uri.c b/src/spice-uri.c
> index 0cc2195..35d773e 100644
> --- a/src/spice-uri.c
> +++ b/src/spice-uri.c
> @@ -141,6 +141,9 @@ gboolean spice_uri_parse(SpiceURI *self, const gchar
> *_uri, GError **error)
>          spice_uri_set_user(self, user);
>          spice_uri_set_password(self, pass);
>          uri = next;
> +    } else {
> +        spice_uri_set_user(self, NULL);
> +        spice_uri_set_password(self, NULL);
>      }
>  

Why not setting all to NULL at the beginning ?
Does the internal state has to be unchanged if the function returns FALSE ?

>      /* max 2 parts, host:port */
> diff --git a/tests/test-spice-uri.c b/tests/test-spice-uri.c
> index e8cfcc9..3d21fa7 100644
> --- a/tests/test-spice-uri.c
> +++ b/tests/test-spice-uri.c
> @@ -36,10 +36,10 @@ static void test_spice_uri_ipv4(void)
>          {"http://127.0.0.1:port", "http", "127.0.0.1", 3128, NULL, NULL}, /*
>          invalid port */
>      };
>      const struct test_case valid_test_cases[] = {
> +        {"http://user:password@host:80", "http", "host", 80, "user",
> "password"},
>          {"http://127.0.0.1/", "http", "127.0.0.1", 3128, NULL, NULL},
>          {"https://127.0.0.1", "https", "127.0.0.1", 3129, NULL, NULL},
>          {"127.0.0.1", "http", "127.0.0.1", 3128, NULL, NULL},
> -        {"http://user:password@host:80", "http", "host", 80, "user",
> "password"},
>      };
>  

I think a comment stating why they are in this order would be good.

Frediano