[Spice-devel] [spice-server] reds: Replace RedsClientMonitorsConfig with SpiceBuffer

Christophe Fergeau cfergeau at redhat.com
Thu Jun 15 09:19:18 UTC 2017


On Wed, Jun 14, 2017 at 01:08:29PM -0400, Frediano Ziglio wrote:
> >  void reds_on_main_agent_data(RedsState *reds, MainChannelClient *mcc, const
> >  void *message,
> 
> Weird name "offset" for a length. Usually I found capacity + len(gth), but
> is not a regression.

Yeah, name is a bit unexpected, but this is how it is at the moment, and
my reading of the SpiceBuffer code makes me think this really is a
length of valid data, and not an offset inside a bigger buffer.

> 
> > @@ -3410,6 +3398,7 @@ static int do_spice_init(RedsState *reds,
> > SpiceCoreInterface *core_interface)
> >      reds->char_devices = NULL;
> >      reds->mig_wait_disconnect_clients = NULL;
> >      reds->vm_running = TRUE; /* for backward compatibility */
> > +    spice_buffer_free(&reds->client_monitors_config);
> >  
> >      if (!(reds->mig_timer = reds->core.timer_add(&reds->core,
> >      migrate_timeout, reds))) {
> >          spice_error("migration timer create failed");
> > @@ -3438,8 +3427,6 @@ static int do_spice_init(RedsState *reds,
> > SpiceCoreInterface *core_interface)
> >  
> >      reds->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> >  
> > -    reds_client_monitors_config_cleanup(reds);
> > -
> >      reds->allow_multiple_clients = getenv(SPICE_DEBUG_ALLOW_MC_ENV) != NULL;
> >      if (reds->allow_multiple_clients) {
> >          spice_warning("spice: allowing multiple client connections");
> 
> Weird to have a cleanup/free call during an init but not a regression.
> Why did you move it to the start of the function?

Moved it so that it's closer to all the initialization. I'll move it
back ;) I assumed this is called here in case _init() is called
multiple times (which we don't do).

> Maybe should also be called by spice_server_destroy.

It's true there could be a small leak if we received a partial monitors
config message, and never the rest of the data.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170615/3ac5c1a0/attachment.sig>


More information about the Spice-devel mailing list