[Spice-devel] [PATCH spice-gtk 7/9] session: allocate in spice_uri_create()

[Christophe Fergeau]

Short log would be more explicit with
session: Return allocated string from spice_uri_create
On Tue, Jan 13, 2015 at 05:38:26PM +0100, Marc-André Lureau wrote:
> This allows more flexible string building.
> ---
>  gtk/spice-session.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index ecc9e86..7872af7 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -353,21 +353,26 @@ spice_session_finalize(GObject *gobject)
>  #define URI_QUERY_START ";?"
>  #define URI_QUERY_SEP   ";&"
>  
> -static int spice_uri_create(SpiceSession *session, char *dest, int len)
> +static gchar* spice_uri_create(SpiceSession *session)
>  {
>      SpiceSessionPrivate *s = session->priv;
> -    int pos = 0;
>  
> -    if (s->host == NULL || (s->port == NULL && s->tls_port == NULL)) {
> -        return 0;
> -    }
> +    if (s->host) {

The "(s->port == NULL && s->tls_port == NULL))" test was lost

> +        GString *str = g_string_new(URI_SCHEME_SPICE);
> +
> +        g_string_append(str, s->host);
> +        g_string_append(str, "?");
> +        if (s->port) {
> +            g_string_append_printf(str, "port=%s&", s->port);
> +        }
> +        if (s->tls_port) {
> +            g_string_append_printf(str, "tls-port=%s", s->tls_port);
> +        }

';' was previously used as a separator between port/tls-port, and the
URI ended with a ';' as well. This patch changes that to '&' and no
trailing ';'

> +        return g_string_free(str, FALSE);
> +    } else
> +        g_return_val_if_reached(NULL);
>  
> -    pos += snprintf(dest + pos, len-pos, "spice://%s?", s->host);
> -    if (s->port && strlen(s->port))
> -        pos += snprintf(dest + pos, len - pos, "port=%s;", s->port);
> -    if (s->tls_port && strlen(s->tls_port))
> -        pos += snprintf(dest + pos, len - pos, "tls-port=%s;", s->tls_port);
> -    return pos;
> +    return NULL;

Maybe deserves a g_warn_if_reached() as getting there is not possible.

Looks much nicer with g_string_append rather than snprintf :)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150127/ab27d2be/attachment.sig>