[Spice-devel] [PATCH 2/2] Revert "Remove use of opaque from vdi_port_read_one_msg_from_device"

[Frediano Ziglio]

> 
> > 
> > This reverts commit 0f947b425cc2219f3c5a8705f4214c2deb607a5b.
> > This patch causes bug as sin->st can be NULL in some cases.
> > opaque is always referring to RedsState.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> 
> It could be weird but I'm not sure this is the right fix to do.
> Why sin->st is reset in the first place?
> I think Qemu is possibly calling spice_remove_interface.
> I tried to came up with a patch:
> 
> diff --git a/server/reds.c b/server/reds.c
> index efd1429..cd1b2fc 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3266,6 +3266,7 @@ static void
> spice_server_char_device_remove_interface(RedsState *reds, SpiceBase
>      if (strcmp(char_device->subtype, SUBTYPE_VDAGENT) == 0) {
>          if (reds->vdagent) {
>              reds_agent_remove(reds);
> +
> red_char_device_reset_dev_instance(RED_CHAR_DEVICE(reds->vdagent),
> NULL);
>          }
>      }
>  #ifdef USE_SMARTCARD
> 
> 
> but in the added line reds->vdagent is already set to NULL (by
> reds_agent_remove).
> 
> Sorry... I just realized the mistake... thank to the (missing) type system...
> the line should be
> 
>    red_char_device_reset_dev_instance(RED_CHAR_DEVICE(reds->agent_dev),
>    NULL);
> 

Hi,
  Eduardo confirmed that all tests passed with this change.

I would like to have some opinion on the best approach to follow.

Frediano

> >  server/reds.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index 67c262a..ab35aa7 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -830,14 +830,12 @@ static void vdi_port_read_buf_free(RedVDIReadBuf
> > *buf)
> >  static RedPipeItem
> >  *vdi_port_read_one_msg_from_device(SpiceCharDeviceInstance *sin,
> >                                                        void *opaque)
> >  {
> > -    RedsState *reds;
> > -    RedCharDeviceVDIPort *dev = RED_CHAR_DEVICE_VDIPORT(sin->st);
> > +    RedsState *reds = opaque;
> > +    RedCharDeviceVDIPort *dev = reds->agent_dev;
> >      SpiceCharDeviceInterface *sif;
> >      RedVDIReadBuf *dispatch_buf;
> >      int n;
> >  
> > -    g_object_get(dev, "spice-server", &reds, NULL);
> > -    g_assert(RED_CHAR_DEVICE(reds->agent_dev) == sin->st);
> >      if (!reds->vdagent) {
> >          return NULL;
> >      }
> > --
> > 2.5.5
> > 
> > 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>