Re: patch[1/1] fix a memory leak in qxl_screen_init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
To: "bigclouds" <bigclouds@xxxxxxx>
Cc: spice-devel@xxxxxxxxxxxxxxxxxxxxx
Sent: Wednesday, November 6, 2013 4:28:45 PM
Subject: Re:  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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
>>http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]