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

[Frediano Ziglio]

> 
> Hi,
> 
> On Fri, May 06, 2016 at 12:11:46PM +0200, Victor Toso wrote:
> > Hi,
> > 
> > On Thu, May 05, 2016 at 01:33:05PM +0100, Frediano Ziglio wrote:
> > > This reverts commit 0f947b425cc2219f3c5a8705f4214c2deb607a5b.
> > > This patch causes bug as sin->st can be NULL in some cases.
> > > opaque is always referring to RedsState.
> > >
> > 
> > This patch fixes (avoids?) crash that happens when I reboot a win7 VM;
> > Let me know if you need more info from my side to track the issue.
> > 
> > Acked-by: Victor Toso <victortoso at redhat.com>
> 
> Sorry to jump in with the 'ack'. Crashing is no good but I can see that
> you were discussing the best approach for this.
> 
>   toso
> 

No problem.

Just to recap:
- commit 0f947b425cc2219f3c5a8705f4214c2deb607a5b introduced a bug due
  to sin->st possibly being NULL;
- rolling back solve the issue;
- patch was written by Jonathon;
  - different people acked the patch;
  - Christophe said didn't like the idea of rolling back;
  - I don't like too, looks like we move back and does not make sense
    that st pointer disappears.

So we have 2 solutions that works and we should decide which one is
the best.

Frediano

> > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  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
>