[Spice-devel] [PATCH spice 1/2] reds: Do not crash when setting agent property

[Pavel Grunt]

Hi,

On Wed, 2016-09-07 at 05:10 -0400, Frediano Ziglio wrote:
> > 
> > Agent properties like file transfer or copy & paste can be
> > disabled by
> > calling spice_server_set_agent_{copypaste, file_xfer} before the
> > spice
> > server is initialized. In that case the call crashes the server
> > because
> > the agent device is created after the initialization.
> > 
> > To avoid the crash this commit introduce a helper function for
> > setting
> > the agent properties after the server is initialized.
> > ---
> >  server/reds.c | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index 74f7727..90d04c8 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -730,6 +730,18 @@ static void reds_update_mouse_mode(RedsState
> > *reds)
> >      }
> >  }
> >  
> > +static void reds_update_agent_properties(RedsState *reds)
> > +{
> > +    if (reds->agent_dev == NULL || reds->config == NULL)
> > +        return;
> 
> 
> Brackets
> 
> > +    /* copy & paste */
> > +    reds->agent_dev->priv->write_filter.copy_paste_enabled =
> > reds->config->agent_copypaste;
> > +    reds->agent_dev->priv->read_filter.copy_paste_enabled =
> > reds->config->agent_copypaste;
> > +    /* file transfer */
> > +    reds->agent_dev->priv->write_filter.file_xfer_enabled =
> > reds->config->agent_file_xfer;
> > +    reds->agent_dev->priv->read_filter.file_xfer_enabled =
> > reds->config->agent_file_xfer;
> > +}
> > +
> >  static void reds_agent_remove(RedsState *reds)
> >  {
> >      // TODO: agent is broken with multiple clients. also need to
> > figure out
> >      what to do when
> > @@ -3438,6 +3450,7 @@ static int do_spice_init(RedsState *reds,
> > SpiceCoreInterface *core_interface)
> >      reds->listen_socket = -1;
> >      reds->secure_listen_socket = -1;
> >      reds->agent_dev = red_char_device_vdi_port_new(reds);
> > +    reds_update_agent_properties(reds);
> >      ring_init(&reds->clients);
> >      reds->num_clients = 0;
> >      reds->main_dispatcher = main_dispatcher_new(reds, reds-
> > >core);
> > @@ -4030,16 +4043,14 @@ SPICE_GNUC_VISIBLE int
> > spice_server_set_agent_mouse(SpiceServer *reds, int enabl
> >  SPICE_GNUC_VISIBLE int
> > spice_server_set_agent_copypaste(SpiceServer *reds,
> >  int enable)
> >  {
> >      reds->config->agent_copypaste = enable;
> > -    reds->agent_dev->priv->write_filter.copy_paste_enabled =
> > reds->config->agent_copypaste;
> > -    reds->agent_dev->priv->read_filter.copy_paste_enabled =
> > reds->config->agent_copypaste;
> > +    reds_update_agent_properties(reds);
> >      return 0;
> >  }
> >  
> >  SPICE_GNUC_VISIBLE int
> > spice_server_set_agent_file_xfer(SpiceServer *reds,
> >  int enable)
> >  {
> >      reds->config->agent_file_xfer = enable;
> > -    reds->agent_dev->priv->write_filter.file_xfer_enabled =
> > reds->config->agent_file_xfer;
> > -    reds->agent_dev->priv->read_filter.file_xfer_enabled =
> > reds->config->agent_file_xfer;
> > +    reds_update_agent_properties(reds);
> >      return 0;
> >  }
> >  
> 
> 
> Looks like read and write filters can be set independently so this
> patch seems to change a bit behavior but as you said if these
> functions
> are meant to be called only during initialization they are fine.
Qemu uses them only during before init, but it is not documented when
they should be called. I would say anytime. In the past/stable it
worked anytime, now it works only after init.

> 
> Is this another regression caused by some Agent change?

It is, but I didn't look for the commit.

> 
> Beside the style,
> 
> Acked-by: Frediano Ziglio <fziglio at redhat.com>
> 
> Frediano
>