[Spice-devel] [PATCH spice-server 1/9] reds: Use GLib memory functions for RedVDIReadBuf

[Victor Toso]

Hi,

On Tue, Dec 05, 2017 at 04:28:52AM -0500, Frediano Ziglio wrote:
> > 
> > On Tue, Dec 05, 2017 at 04:14:10AM -0500, Frediano Ziglio wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > On Tue, Dec 05, 2017 at 08:41:04AM +0000, Frediano Ziglio wrote:
> > > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > 
> > > > > ---
> > > > >  server/reds.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/server/reds.c b/server/reds.c
> > > > > index 64b3a967..4e652253 100644
> > > > > --- a/server/reds.c
> > > > > +++ b/server/reds.c
> > > > > @@ -4519,7 +4519,7 @@
> > > > > red_char_device_vdi_port_init(RedCharDeviceVDIPort
> > > > > *self)
> > > > >      self->priv->receive_len = sizeof(self->priv->vdi_chunk_header);
> > > > >  
> > > > >      for (i = 0; i < REDS_VDI_PORT_NUM_RECEIVE_BUFFS; i++) {
> > > > > -        RedVDIReadBuf *buf = spice_new0(RedVDIReadBuf, 1);
> > > > > +        RedVDIReadBuf *buf = g_new0(RedVDIReadBuf, 1);
> > > > 
> > > > Yes, last one in server
> > > > Acked-by: Victor Toso <victortoso at redhat.com>
> > > > 
> > > 
> > > Yes, this was my last patch in my glib_memory branch (beside other OT
> > > ones).
> > > Note that surely this identify a leak which was not detected by either
> > > Valgrind nor ASan. Looks like the circular pointers
> > > Agent -> RedVDIReadBuf -> Agent made these tools believe there was
> > > no leak :-(
> > 
> > Interesting! How you manage to see the leak without Valgrind/ASan to
> > point it out to you?
> > 
> 
> It's simple, as you can see there's a malloc (in this case spice_new0)
> without a free.

Ah, so it was code introspection.

> I think when you release the agent_dev the tools starts seeing the
> leaks as you remove the circular reference.

Right :)

Cheers,

> 
> > > (I think I spent half an hours of compiling and testing to understand
> > > this).
> > > 
> > > > >          vdi_read_buf_init(buf);
> > > > >          buf->dev = self;
> > > > >          g_warn_if_fail(!self->priv->agent_attached);
> > > 
> > > Frediano
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20171205/b4f5cca7/attachment.sig>