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