On Son, 2013-07-28 at 18:05 +0200, Julien Cristau wrote: > On Thu, Jul 11, 2013 at 18:27:14 -0400, Alex Deucher wrote: > > On Thu, Jul 11, 2013 at 6:02 PM, Julien Cristau <[email protected]> wrote: > > > On Mon, Jun 17, 2013 at 12:39:20 -0400, [email protected] wrote: > > > > > >> From: Leo Liu <[email protected]> > > >> > > >> leak happens when looping xrandr prop. > > >> > > >> Signed-off-by: Leo Liu <[email protected]> > > >> --- > > >> hw/xfree86/common/xf86Helper.c | 1 + > > >> 1 file changed, 1 insertion(+) > > >> > > >> diff --git a/hw/xfree86/common/xf86Helper.c > > >> b/hw/xfree86/common/xf86Helper.c > > >> index 721159d..bb17ecc 100644 > > >> --- a/hw/xfree86/common/xf86Helper.c > > >> +++ b/hw/xfree86/common/xf86Helper.c > > >> @@ -1813,6 +1813,7 @@ xf86RegisterRootWindowProperty(int ScrnIndex, Atom > > >> property, Atom type, > > >> } > > >> else { > > >> free(pNewProp->name); > > >> + free(pNewProp->data); > > >> existing = TRUE; > > >> } > > >> > > > AFAICT xf86RegisterRootWindowProperty can be called with non-malloc > > > 'value' (e.g. for the SeatId stuff in InitOutput, although that's only > > > for serverGeneration == 1). Is there any way to make this a little more > > > obviously correct? > > > > How about: > > > > else { > > free(pNewProp->name); > > + if(pNewProp->data) > > + free(pNewProp->data); > > existing = TRUE; > > > That doesn't really change anything. I was thinking more of making sure > that function is always called with malloced data.
Leo implemented the attached two approaches which hopefully address your concern. Which of these would be preferable, or did you think of yet another approach? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Debian, X and DRI developer
From 607d987d838d3ba1f7a15a72932e682f18225d71 Mon Sep 17 00:00:00 2001 From: Leo Liu <[email protected]> Date: Wed, 31 Jul 2013 09:57:25 -0400 Subject: [PATCH] xfree86: fix a memory leak in edidMakeAtom(). leak happens when looping xrandr prop. Signed-off-by: Leo Liu <[email protected]> --- hw/xfree86/common/xf86Helper.c | 6 +++++- hw/xfree86/common/xf86Init.c | 1 + hw/xfree86/ddc/ddcProperty.c | 7 +------ 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index 721159d..ed8cc87 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -1813,6 +1813,7 @@ xf86RegisterRootWindowProperty(int ScrnIndex, Atom property, Atom type, } else { free(pNewProp->name); + free(pNewProp->data); existing = TRUE; } @@ -1820,7 +1821,10 @@ xf86RegisterRootWindowProperty(int ScrnIndex, Atom property, Atom type, pNewProp->type = type; pNewProp->format = format; pNewProp->size = len; - pNewProp->data = value; + if (!(pNewProp->data = (pointer)malloc(len * format/8))) + return BadAlloc; + memcpy(pNewProp->data, value, (len * format/8)); DebugF("new property filled\n"); diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index 91ec4c8..9694ab4 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -747,6 +747,7 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv) xf86DrvMsg(xf86Screens[i]->scrnIndex, X_WARNING, "Failed to register VT property\n"); } + free(VT); } if (SeatId) { diff --git a/hw/xfree86/ddc/ddcProperty.c b/hw/xfree86/ddc/ddcProperty.c index fc63f0e..16dd476 100644 --- a/hw/xfree86/ddc/ddcProperty.c +++ b/hw/xfree86/ddc/ddcProperty.c @@ -38,14 +38,9 @@ static void edidMakeAtom(int i, const char *name, CARD8 *data, int size) { Atom atom; - unsigned char *atom_data; - - if (!(atom_data = malloc(size * sizeof(CARD8)))) - return; atom = MakeAtom(name, strlen(name), TRUE); - memcpy(atom_data, data, size); - xf86RegisterRootWindowProperty(i, atom, XA_INTEGER, 8, size, atom_data); + xf86RegisterRootWindowProperty(i, atom, XA_INTEGER, 8, size, data); } static void -- 1.7.9.5
From 9e2d6275373b8b5a729120cf9405bd8ae6ac39dd Mon Sep 17 00:00:00 2001 From: Leo Liu <[email protected]> Date: Wed, 31 Jul 2013 10:16:20 -0400 Subject: [PATCH] xfree86: fix a memory leak in edidMakeAtom(). leak happens when looping xrandr prop. Signed-off-by: Leo Liu <[email protected]> --- hw/xfree86/common/xf86Helper.c | 1 + hw/xfree86/common/xf86Init.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index 721159d..bb17ecc 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -1813,6 +1813,7 @@ xf86RegisterRootWindowProperty(int ScrnIndex, Atom property, Atom type, } else { free(pNewProp->name); + free(pNewProp->data); existing = TRUE; } diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index 91ec4c8..cdc7322 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -751,17 +751,18 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv) if (SeatId) { Atom SeatAtom; + char *SeatId_data; SeatAtom = MakeAtom(SEAT_ATOM_NAME, sizeof(SEAT_ATOM_NAME) - 1, TRUE); - + SeatId_data = xnfstrdup(SeatId); for (i = 0; i < xf86NumScreens; i++) { int ret; ret = xf86RegisterRootWindowProperty(xf86Screens[i]->scrnIndex, SeatAtom, XA_STRING, 8, strlen(SeatId) + 1, - SeatId); + SeatId_data); if (ret != Success) { xf86DrvMsg(xf86Screens[i]->scrnIndex, X_WARNING, "Failed to register seat property\n"); -- 1.7.9.5
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
