[Spice-devel] [v2: PATCH 1/3] Add SpiceChannelError

[Marc-André Lureau]

----- Original Message -----
> 
> 
> On Wed, Oct 8, 2014 at 11:06 AM, Marc-André Lureau <
> marcandre.lureau at gmail.com > wrote:
> 
> 
> 
> 
> 
> On Mon, Oct 6, 2014 at 4:23 PM, Christophe Fergeau < cfergeau at redhat.com >
> wrote:
> 
> 
> ACK
> 
> On Mon, Oct 06, 2014 at 01:52:43PM +0200, Fabiano Fidêncio wrote:
> > SpiceChannelError, for now, will be used to set more detailed errors
> > with respect to the main channel event SPICE_CHANNEL_ERROR_AUTH.
> > ---
> > Changes since v1:
> > - Change the SpiceChannelAuthError comments to match with the enum values
> > - Add more details related to SPICE_CHANNEL_ERROR_AUTH
> > ---
> > doc/reference/spice-gtk-sections.txt | 3 +++
> > gtk/map-file | 2 ++
> > gtk/spice-channel.c | 11 +++++++++++
> > gtk/spice-channel.h | 25 +++++++++++++++++++++++++
> > gtk/spice-glib-sym-file | 2 ++
> > 5 files changed, 43 insertions(+)
> > 
> > diff --git a/doc/reference/spice-gtk-sections.txt
> > b/doc/reference/spice-gtk-sections.txt
> > index caaa92c..5c411f8 100644
> > --- a/doc/reference/spice-gtk-sections.txt
> > +++ b/doc/reference/spice-gtk-sections.txt
> > @@ -73,6 +73,9 @@ spice_main_clipboard_release
> > spice_main_clipboard_notify
> > spice_main_clipboard_request
> > <SUBSECTION Standard>
> > +SPICE_CHANNEL_ERROR
> > +spice_channel_error_get_type
> > +spice_channel_error_quark
> > SPICE_MAIN_CHANNEL
> > SPICE_IS_MAIN_CHANNEL
> > SPICE_TYPE_MAIN_CHANNEL
> > diff --git a/gtk/map-file b/gtk/map-file
> > index 90f14f1..c1d9e71 100644
> > --- a/gtk/map-file
> > +++ b/gtk/map-file
> > @@ -6,6 +6,8 @@ spice_audio_new;
> > spice_channel_connect;
> > spice_channel_destroy;
> > spice_channel_disconnect;
> > +spice_channel_error_get_type;
> > +spice_channel_error_quark;
> > spice_channel_event_get_type;
> > spice_channel_get_error;
> > spice_channel_get_type;
> > diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> > index 5d1a86e..a8b4e35 100644
> > --- a/gtk/spice-channel.c
> > +++ b/gtk/spice-channel.c
> > @@ -76,6 +76,17 @@ static void
> > spice_channel_send_migration_handshake(SpiceChannel *channel);
> > 
> > G_DEFINE_TYPE(SpiceChannel, spice_channel, G_TYPE_OBJECT);
> > 
> > +/**
> > + * SpiceChannelError:
> > + *
> > + * The error domain for the spice channel.
> > + **/
> > +GQuark
> > +spice_channel_error_quark(void)
> > +{
> > + return g_quark_from_static_string ("spice-channel-error");
> > +}
> > +
> > /* Properties */
> > enum {
> > PROP_0,
> > diff --git a/gtk/spice-channel.h b/gtk/spice-channel.h
> > index 1c303b4..adbe8f5 100644
> > --- a/gtk/spice-channel.h
> > +++ b/gtk/spice-channel.h
> > @@ -32,6 +32,17 @@ G_BEGIN_DECLS
> > #define SPICE_IS_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass),
> > SPICE_TYPE_CHANNEL))
> > #define SPICE_CHANNEL_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj),
> > SPICE_TYPE_CHANNEL, SpiceChannelClass))
> > 
> > +/**
> > + * SPICE_CHANNEL_ERROR:
> > + *
> > + * Error domain for #SpiceChannel operations. Error in this domain will be
> > + * from the SpiceChannelError enumeration. See #GError for more
> > + * information on error domains.
> > + **/
> > +#define SPICE_CHANNEL_ERROR spice_channel_error_quark ()
> > +
> 
> SPICE_CHANNEL_ERROR_* is unfortunately already used in SpiceChannelEvent
> enum.
> 
> Yeah, and I'd love to get rid of them at some point, moving them to
> SPICE_CHANNEL_ERROR.

I understand, it's a bit unfortunate that we didn't report channel-event with an optionnal GError.

However I don't think that deserves an API break, so we can keep the current ChannelEvent enum & signal and report errors via get_error(), that was the choice made to not break.

Imho, we should keep that in the TODO list when breaking API. 

> 
> Why not add error values to SpiceClientError instead? This is the generic
> error domain for all spice-gtk I am not sure we need other domains.
> 
> Does not look easier to read/understand than the way it's now, but if you
> prefer, we can do in this way.

Yeah, I would prefer we keep the same GError domain for spice-gtk atm. I don't see a need to split.