[Spice-devel] [PATCH v2 2/2] fix crash if agent interface is removed

Frediano Ziglio fziglio at redhat.com
Tue May 10 13:36:30 UTC 2016


>
> Hey,
> 
> On Mon, May 09, 2016 at 11:20:00AM -0400, Frediano Ziglio wrote:
> > > 
> > > Hey,
> > > 
> > > On Mon, May 09, 2016 at 10:03:03AM +0100, Frediano Ziglio wrote:
> > > > Removing an interface cause SpiceBaseInstance->st to be set to NULL.
> > > > This pointer was then deferenced in agent code.
> > > > As SpiceBaseInstance should not be used after this call make sure
> > > > we don't keep pointers to it.
> > > 
> > > I guess it cannot hurt... However, it would be a good time to document
> > > how we expect char_device_attach/remove to behave wrt to the lifetime of
> > > the various instances we have (char device, sin, vdagent, ...).
> > > 
> > > Christophe
> > > 
> > 
> > Do you have any suggestion on where to document this?
> 
> Some new text file in a docs/ subdirectory?
>

Personally I prefer to use Doxygen, external documentation tend
to be out of date compared to inline documentation.

I know we use some gnome inline documentation style but honestly
I think it's much less powerful than Doxygen (or I don't know
gnome documentation that much).
 
> > Do you think this is a stopover for the patch?
> 
> If these 'rules' are fairly clear in your mind as you just worked on it,
> it would be good to do a braindump now yeah as it will be more work
> later. If you already don't quite remember, then no big deal.
> 

The lifespan of a "SpiceBaseInterface" for spice-server is from
spice_server_add_interface to  spice_server_remove_interface.

> > Do you think the fix should be done in another way?
> 
> Unfortunately it has been a long time that I looked at that code, so I
> don't really have alternative suggestions :(
> 
> Christophe
> 

I'll post another version with code comment(s).

Frediano


More information about the Spice-devel mailing list