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

[Jeremy White]

On 03/26/2014 11:30 AM, longguang.yue wrote:
> please forgive my ignore.
>
> jwhite you are right, i post a new patch as you suggest.

ACKed

(and pushed)

Thanks!

Jeremy

> thanks
>
>
>
>
>
>
> At 2013-11-19 20:21:35,"Marian Krcmarik" <mkrcmari at redhat.com> wrote:
>>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
>>>
>
>
>