[Spice-devel] [PATCH] Fixed error when trying to access undefined source buffer without checking it first

[Oliver Gutierrez]

The problem I was having was the spice-html5 client was throwing exceptions
for trying to access that property in cases where it is not a valid value.

I got to the error because I'm developing a plugin for cockpit that makes
use of spice-html5 for connecting to VMs, and when spice-html5 starts
displaying graphics, it throws this error repeatedly, causing cockpit to
show an awfull "Oops" message.

It just failed throwing exceptions to console. It stopped the playback when
failed, so with the change it stops anyway but without throwing errors.

I hope this answer your questions. My knowledge about spice-html5 or spice
is limited, so I don't know if this info is enough for you.

On Thu, Jul 28, 2016 at 12:47 PM, Christophe Fergeau <cfergeau at redhat.com>
wrote:

> Hey,
>
> Thanks for the patch!
>
> I'd expand a bit in the commit log, that this.source_buffer can be used
> before being checked for null, and that this commit moves the check
> before the first use of this.source_buffer.
> You could also describe what happens when this triggers/how this
> triggers (I assume playback stops?)
>
> On Thu, Jul 28, 2016 at 10:50:05AM +0100, Oliver Gutierrez wrote:
> > ---
> >  playback.js | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/playback.js b/playback.js
> > index 9659381..b5954da 100644
> > --- a/playback.js
> > +++ b/playback.js
> > @@ -107,21 +107,20 @@
> SpicePlaybackConn.prototype.process_channel_message = function(msg)
> >               So we do two things.  First, we seek forward.  Second, we
> compute how much of a gap
> >               there would have been, and essentially eliminate it.
> >          */
> > +        if (! this.source_buffer)
> > +            return true;
> > +
> >          if (this.last_data_time && data.time >= (this.last_data_time +
> GAP_DETECTION_THRESHOLD))
> >          {
> >              this.skip_until = data.time;
> > -            this.gap_time = (data.time - this.start_time) -
> > +            this.gap_time = (data.time - this.start_time) -
>
> This looks like an unrelated whitespace change
>
>
> Looks good otherwise!
>
> Reviewed-by: Christophe Fergeau <cfergeau at redhat.com>
>
> Christophe
>



-- 
Oliver Gutierrez
Associate Software Engineer - Desktop Management tools
Red Hat
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160728/56f64a65/attachment.html>