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

[Pavel Grunt]

On Thu, 2016-05-19 at 06:24 -0400, Frediano Ziglio wrote:
> > 
> > 
> > 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 ?

Sure
> Does the internal state has to be unchanged if the function returns FALSE ?

I don't think so
> 
> >      /* 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.

Right, it was in previous version.

Pavel