On Wed, Feb 12, 2014 at 11:48 AM, Christophe Fergeau
<cfergeau@xxxxxxxxxx> wrote:
> On Tue, Feb 11, 2014 at 12:37:53PM -0500, Marc-André Lureau wrote:
>> > > static void
>> > > wocky_http_proxy_init (WockyHttpProxy *proxy)
>> > > @@ -180,10 +182,34 @@ wocky_http_proxy_connect (GProxy *proxy,
>> > > {
>> > > GInputStream *in;
>> > > GOutputStream *out;
>> > > - GDataInputStream *data_in;
>> > > - gchar *buffer;
>> > > + GDataInputStream *data_in = NULL;
>> > > + gchar *buffer = NULL;
>> > > gboolean has_cred;
>> > >
>> > > +#if GLIB_CHECK_VERSION(2, 28, 0)
>> > > + if (WOCKY_IS_HTTPS_PROXY (proxy))
>> >
>> > Having runtime type checks in a interface virtual method is not very nice,
>> > having a wocky_https_proxy_iface_init would allow to call directly the
>> > right implementation without having to resort to runtime type checking.
>>
>> I decided to use this consciously, as doing it differently added much
>> more boilerplate for no clear benefit.
>
> Gave it a quick try (only compile-tested), and the resulting code is not
> particularly bad. Having both inheritance and runtime type checks is imo
> quite ugly. I can send a follow-up patch after your series land if it's likely
> to be accepted.
Sure, I'll review then (the patch below looks incomplete from a brief
view). I still prefer a runtime type check for this final case.
>
>
> diff --git a/gtk/wocky-http-proxy.c b/gtk/wocky-http-proxy.c
> index 25f2af5..3eb6931 100644
> --- a/gtk/wocky-http-proxy.c
> +++ b/gtk/wocky-http-proxy.c
> @@ -186,30 +186,6 @@ wocky_http_proxy_connect (GProxy *proxy,
> gchar *buffer = NULL;
> gboolean has_cred;
>
> -#if GLIB_CHECK_VERSION(2, 28, 0)
> - if (WOCKY_IS_HTTPS_PROXY (proxy))
> - {
> - GIOStream *tlsconn;
> -
> - tlsconn = g_tls_client_connection_new (io_stream,
> - G_SOCKET_CONNECTABLE(proxy_address),
> - error);
> - if (!tlsconn)
> - goto error;
> -
> - GTlsCertificateFlags tls_validation_flags = G_TLS_CERTIFICATE_VALIDATE_ALL;
> -#ifdef DEBUG
> - tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA + G_TLS_CERTIFICATE_BAD_IDENTITY;
> -#endif
> - g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (tlsconn),
> - tls_validation_flags);
> - if (!g_tls_connection_handshake (G_TLS_CONNECTION (tlsconn), cancellable, error))
> - goto error;
> -
> - io_stream = tlsconn;
> - }
> -#endif
> -
> in = g_io_stream_get_input_stream (io_stream);
> out = g_io_stream_get_output_stream (io_stream);
>
> @@ -251,6 +227,41 @@ error:
> return NULL;
> }
>
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +static GIOStream *
> +wocky_https_proxy_connect (GProxy *proxy,
> + GIOStream *io_stream,
> + GProxyAddress *proxy_address,
> + GCancellable *cancellable,
> + GError **error)
> +{
> + {
> + GIOStream *tlsconn;
> +
> + tlsconn = g_tls_client_connection_new (io_stream,
> + G_SOCKET_CONNECTABLE(proxy_address),
> + error);
> + if (!tlsconn)
> + goto error;
> +
> + GTlsCertificateFlags tls_validation_flags = G_TLS_CERTIFICATE_VALIDATE_ALL;
> +#ifdef DEBUG
> + tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA + G_TLS_CERTIFICATE_BAD_IDENTITY;
> +#endif
> + g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (tlsconn),
> + tls_validation_flags);
> + if (!g_tls_connection_handshake (G_TLS_CONNECTION (tlsconn), cancellable, error))
> + goto error;
> +
> + io_stream = tlsconn;
> + }
> + return wocky_http_proxy_connect(proxy, io_stream, proxy_address, cancellable, error);
> +
> +error:
> + return NULL;
> +}
> +#endif
> +
>
> typedef struct
> {
> @@ -351,8 +362,9 @@ handshake_completed (GObject *source_object,
> }
> #endif
>
> -static void
> -wocky_http_proxy_connect_async (GProxy *proxy,
> + static ConnectAsyncData *
> + wocky_http_proxy_connect_async_data_new (
> + GProxy *proxy,
> GIOStream *io_stream,
> GProxyAddress *proxy_address,
> GCancellable *cancellable,
> @@ -364,7 +376,7 @@ wocky_http_proxy_connect_async (GProxy *proxy,
>
> simple = g_simple_async_result_new (G_OBJECT (proxy),
> callback, user_data,
> - wocky_http_proxy_connect_async);
> + wocky_http_proxy_connect_async_data_new);
>
> data = g_slice_new0 (ConnectAsyncData);
> if (cancellable != NULL)
> @@ -378,12 +390,30 @@ wocky_http_proxy_connect_async (GProxy *proxy,
> g_simple_async_result_set_op_res_gpointer (simple, data,
> (GDestroyNotify) free_connect_data);
>
> + return data;
> + }
> +
> +
> #if GLIB_CHECK_VERSION(2, 28, 0)
> - if (WOCKY_IS_HTTPS_PROXY (proxy))
> +static void
> +wocky_https_proxy_connect_async (GProxy *proxy,
> + GIOStream *io_stream,
> + GProxyAddress *proxy_address,
> + GCancellable *cancellable,
> + GAsyncReadyCallback callback,
> + gpointer user_data)
> {
> + ConnectAsyncData *data;
> GError *error = NULL;
> GIOStream *tlsconn;
>
> + data = wocky_http_proxy_connect_async_data_new(proxy,
> + io_stream,
> + proxy_address,
> + cancellable,
> + callback,
> + user_data);
> +
> tlsconn = g_tls_client_connection_new (io_stream,
> G_SOCKET_CONNECTABLE(proxy_address),
> &error);
> @@ -405,12 +435,26 @@ wocky_http_proxy_connect_async (GProxy *proxy,
> G_PRIORITY_DEFAULT, cancellable,
> handshake_completed, data);
> }
> - else
> #endif
> +
> +static void
> +wocky_http_proxy_connect_async (GProxy *proxy,
> + GIOStream *io_stream,
> + GProxyAddress *proxy_address,
> + GCancellable *cancellable,
> + GAsyncReadyCallback callback,
> + gpointer user_data)
> {
> + ConnectAsyncData *data;
> + data = wocky_http_proxy_connect_async_data_new(proxy,
> + io_stream,
> + proxy_address,
> + cancellable,
> + callback,
> + user_data);
> +
> stream_connected(data, io_stream);
> }
> -}
>
> static void
> request_write_cb (GObject *source,
> @@ -510,6 +554,16 @@ wocky_http_proxy_iface_init (GProxyInterface *proxy_iface)
> proxy_iface->supports_hostname = wocky_http_proxy_supports_hostname;
> }
>
> +static void
> +wocky_https_proxy_iface_init (GProxyInterface *proxy_iface)
> +{
> + proxy_iface->connect = wocky_https_proxy_connect;
> + proxy_iface->connect_async = wocky_https_proxy_connect_async;
> + proxy_iface->connect_finish = wocky_http_proxy_connect_finish;
> + proxy_iface->supports_hostname = wocky_http_proxy_supports_hostname;
> +}
> +
> +
> #if GLIB_CHECK_VERSION(2, 28, 0)
> struct _WockyHttpsProxy
> {
> @@ -524,7 +578,7 @@ struct _WockyHttpsProxyClass
> #define wocky_https_proxy_get_type _wocky_https_proxy_get_type
> G_DEFINE_TYPE_WITH_CODE (WockyHttpsProxy, wocky_https_proxy, WOCKY_TYPE_HTTP_PROXY,
> G_IMPLEMENT_INTERFACE (G_TYPE_PROXY,
> - wocky_http_proxy_iface_init)
> + wocky_https_proxy_iface_init)
> g_io_extension_point_set_required_type (
> g_io_extension_point_register (G_PROXY_EXTENSION_POINT_NAME),
> G_TYPE_PROXY);
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel