Re: [spice-gtk 12/13] channel: add spice_channel_get_error()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 11, 2014 at 06:50:18PM +0100, Marc-André Lureau wrote:
> >> @@ -2248,15 +2269,16 @@ static void *spice_channel_coroutine(void *data)
> >>
> >>
> >>  reconnect:
> >> -    c->conn = spice_session_channel_open_host(c->session, channel, &c->tls);
> >> +    c->conn = spice_session_channel_open_host(c->session, channel, &c->tls, &c->error);
> >>      if (c->conn == NULL) {
> >> -        if (!c->tls) {
> >> +        if (!c->error && !c->tls) {
> >>              CHANNEL_DEBUG(channel, "trying with TLS port");
> >>              c->tls = true; /* FIXME: does that really work with provided fd */
> >>              goto reconnect;
> >>          } else {
> >>              CHANNEL_DEBUG(channel, "Connect error");
> >>              emit_main_context(channel, SPICE_CHANNEL_EVENT, SPICE_CHANNEL_ERROR_CONNECT);
> >> +            g_clear_error(&c->error);
> >
> > My understanding of this bit is that spice_channel_get_error() can only be
> > used in a callback attached to SPICE_CHANNEL_EVENT as after that it will be
> > cleared? Maybe this needs to be more explicit in the API doc for
> > spice_channel_get_error().
> 
> It is meant to be usable at any point.

Won't the g_clear_error() I quoted cause spice_channel_get_error() to
return the correct error in the SPICE_CHANNEL_EVENT callback when there is
a connection error, and then return NULL after the g_clear_error() has been
called? Ie the error state of the channel will not be permanent after an
error has happened, but will be silently reset by the library.
Not saying this is bad per-se, at first I was worried that we could get in
a permanent error state it's not possible to get out of.

Christophe

Attachment: pgpn1oAgAt0sl.pgp
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]