[Spice-devel] [PATCH spice-gtk] Add SpiceSession:proxy

[Christophe Fergeau]

On Tue, Jan 29, 2013 at 01:33:43PM +0100, Marc-André Lureau wrote:
> Add a session property to set proxy setting, since it is racy to rely
> on setenv(). Also doing so would override system environment, which
> will modify other session too sharing the same process.
> 
> v2:
> - keep old proxy if update_proxy() fails
> - remove strlen() usage
> - return NULL in spice_proxy_to_string() on incomplete details
> - update property comment

NB: these v2: notes should not be part of what is pushed to the git
repository as they are only useful during review, they are generally added
with git send-email --annotate below the '---' above to make that explicit.

> ---
>  gtk/spice-proxy.c   | 13 +++++++++++++
>  gtk/spice-proxy.h   |  1 +
>  gtk/spice-session.c | 52 ++++++++++++++++++++++++++++++++++++++++++----------
>  3 files changed, 56 insertions(+), 10 deletions(-)
> 
> diff --git a/gtk/spice-proxy.c b/gtk/spice-proxy.c
> index 97c3a6b..581f030 100644
> --- a/gtk/spice-proxy.c
> +++ b/gtk/spice-proxy.c
> @@ -234,3 +234,16 @@ static void spice_proxy_class_init(SpiceProxyClass *klass)
>                                                         G_PARAM_STATIC_STRINGS |
>                                                         G_PARAM_READWRITE));
>  }
> +
> +gchar* spice_proxy_to_string(SpiceProxy* self)
> +{
> +    SpiceProxyPrivate *p;
> +
> +    g_return_val_if_fail(SPICE_IS_PROXY(self), NULL);
> +    p = self->priv;
> +
> +    if (p->protocol == NULL || p->hostname == NULL)
> +        return NULL;
> +
> +    return g_strdup_printf("%s://%s:%u", p->protocol, p->hostname, p->port);
> +}
> diff --git a/gtk/spice-proxy.h b/gtk/spice-proxy.h
> index c780931..1e7b6d7 100644
> --- a/gtk/spice-proxy.h
> +++ b/gtk/spice-proxy.h
> @@ -53,6 +53,7 @@ const gchar* spice_proxy_get_hostname(SpiceProxy* self);
>  void spice_proxy_set_hostname(SpiceProxy* self, const gchar* value);
>  guint spice_proxy_get_port(SpiceProxy* self);
>  void spice_proxy_set_port(SpiceProxy* self, guint port);
> +gchar *spice_proxy_to_string(SpiceProxy* self);
>  
>  G_END_DECLS
>  
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index 5beb6a1..299bebc 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -106,6 +106,7 @@ enum {
>      PROP_UUID,
>      PROP_NAME,
>      PROP_CA,
> +    PROP_PROXY
>  };
>  
>  /* signals */
> @@ -118,24 +119,29 @@ enum {
>  static guint signals[SPICE_SESSION_LAST_SIGNAL];
>  
>  
> -static SpiceProxy* get_proxy(void)
> +static void update_proxy(SpiceSession *self, const gchar *str)
>  {
> +    SpiceSessionPrivate *s = self->priv;
> +    SpiceProxy *proxy = NULL;
>      GError *error = NULL;
> -    SpiceProxy *proxy;
>  
> -    const gchar *proxy_env = g_getenv("SPICE_PROXY");
> -    if (proxy_env == NULL || strlen(proxy_env) == 0)
> -        return NULL;
> +    if (str == NULL)
> +        str = g_getenv("SPICE_PROXY");
> +    if (str == NULL || *str == 0)
> +        return;

We need a g_clear_object(&s->proxy); or we won't be able to clear the
proxy.

>  
>      proxy = spice_proxy_new();
> -    if (!spice_proxy_parse(proxy, proxy_env, &error))
> +    if (!spice_proxy_parse(proxy, str, &error))
>          g_clear_object(&proxy);
>      if (error) {
>          g_warning ("%s", error->message);
>          g_clear_error (&error);
>      }
>  
> -    return proxy;
> +    if (proxy != NULL) {
> +        g_clear_object(&s->proxy);
> +        s->proxy = proxy;
> +    }
>  }
>  
>  static void spice_session_init(SpiceSession *session)
> @@ -167,7 +173,7 @@ static void spice_session_init(SpiceSession *session)
>      cache_init(&s->images, "image");
>      cache_init(&s->palettes, "palette");
>      s->glz_window = glz_decoder_window_new();
> -    s->proxy = get_proxy();
> +    update_proxy(session, NULL);
>  }
>  
>  static void
> @@ -513,6 +519,9 @@ static void spice_session_get_property(GObject    *gobject,
>      case PROP_UUID:
>          g_value_set_pointer(value, s->uuid);
>  	break;
> +    case PROP_PROXY:
> +        g_value_take_string(value, spice_proxy_to_string(s->proxy));
> +	break;
>      default:
>  	G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>  	break;
> @@ -628,6 +637,9 @@ static void spice_session_set_property(GObject      *gobject,
>          g_clear_pointer(&s->ca, g_byte_array_unref);
>          s->ca = g_value_dup_boxed(value);
>          break;
> +    case PROP_PROXY:
> +        update_proxy(session, g_value_get_string(value));
> +        break;
>      default:
>          G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
>          break;
> @@ -1119,6 +1131,23 @@ static void spice_session_class_init(SpiceSessionClass *klass)
>                                G_PARAM_READABLE |
>                                G_PARAM_STATIC_STRINGS));
>  
> +    /**
> +     * SpiceSession:proxy:
> +     *
> +     * URI to the proxy server to use when doing network connection.
> +     * of the form <![CDATA[ [protocol://]<host>[:port] ]]>
> +     *
> +     * Since: 0.17
> +     **/
> +    g_object_class_install_property
> +        (gobject_class, PROP_PROXY,
> +         g_param_spec_string("proxy",
> +                             "Proxy",
> +                             "The proxy server",

Oh, I was referring to the "The proxy server" string, not to the property
comment which was good in the previous patch, sorry for not being clearer
:(

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