[Spice-devel] patch[1/1] fix a memory leak in qxl_screen_init

[Marian Krcmarik]

ping..

"bigclouds",
are you willing to finish the patch and address Jeremy's comment? It's imo worthy to fix and It would be nice If you can finish that.

----- Original Message -----
> From: "Jeremy White" <jwhite at codeweavers.com>
> To: "bigclouds" <bigclouds at 163.com>
> Cc: spice-devel at lists.freedesktop.org
> Sent: Wednesday, November 6, 2013 4:28:45 PM
> Subject: Re: [Spice-devel] patch[1/1] fix a memory leak  in qxl_screen_init
> 
> On 11/06/2013 08:48 AM, bigclouds wrote:
> > it is needed to detect the return of qxl_uxa_init  in qxl_screen_init .
> 
> I don't think we have understood each other.
> 
> There is a duplicate allocation; you are trying to fix that.  As I
> understand it, your patch removes the allocation in qxl_uxa_init and
> leaves only the one in qxl_screen_init.
> 
> I said that it seems to me it would be better to remove the allocation
> in qxl_screen_init and leave only the one in qxl_uxa_init.
> 
> If you're concerned about error conditions propagating, you can change
> qxl_driver to check the return status of qxl_uxa_init.
> 
> Cheers,
> 
> Jeremy
> 
> > 
> > 
> > 
> > 
> > 
> > 
> > At 2013-11-06 21:59:58,"Jeremy White" <jwhite at codeweavers.com> wrote:
> >>Nice catch!
> >>
> >>On 11/06/2013 03:37 AM, bigclouds wrote:
> >>> hi, it allocate twice memory for qxl->uxa in function qxl_screen_init and
> >>> qxl_uxa_init
> >>> -----------------
> >>> diff --git a/src/qxl_driver.c b/src/qxl_driver.c
> >>> index 91ba6c2..6be61e4 100644
> >>> --- a/src/qxl_driver.c
> >>> +++ b/src/qxl_driver.c
> >>> @@ -746,7 +746,9 @@ qxl_screen_init (SCREEN_INIT_ARGS_DECL)
> >>>       }
> >>>       qxl->uxa = uxa_driver_alloc ();
> >>> -
> >>> +    if (qxl->uxa == NULL)
> >>> +return FALSE;
> >>> +
> >>
> >>Wouldn't it be better to just delete this instance of the
> >>allocation and leave it all in qxl_uxa.c?
> >>
> >>Also, just a kibitz, but most open source projects require
> >>a full name on a submitted patch.
> >>
> >>Cheers,
> >>
> >>Jeremy
> >>_______________________________________________
> >>Spice-devel mailing list
> >>Spice-devel at lists.freedesktop.org
> >>http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > 
> > 
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>