[PATCH] xfree86: Copy mode->name for desired mode and current mode
Maarten Lankhorst
maarten.lankhorst at canonical.com
Thu Jun 21 06:49:30 PDT 2012
I noticed that on EnterVT after a suspend valgrind would complain
due to ->name being freed earlier, after X sometimes crashed on
returning from suspend. Attached patch fixes this for me on x1.11
which seems to be identical to x-server.git in almost every way.
It has been reapplied to X server
Alternatively this member could be made a static array instead
of a pointer, but this would require a video abi bump.
I'm still a bit unfamiliar to the core x server so any feedback is welcome.
The specific valgrind error happens a lot later, long after the fact:
==30042== Invalid read of size 1
==30042== at 0x4C2BFD4: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30042== by 0x673E7F5: strdup (strdup.c:42)
==30042== by 0x29673D: XNFstrdup (utils.c:1119)
==30042== by 0x1CEC08: xf86DuplicateMode (xf86Modes.c:209)
==30042== by 0x1C865E: xf86CrtcSetModeTransform (xf86Crtc.c:276)
==30042== by 0x1C8F30: xf86SetDesiredModes (xf86Crtc.c:2674)
==30042== by 0x8C5710E: RADEONEnterVT (radeon_driver.c:6269)
==30042== by 0x1D115F: xf86RandR12EnterVT (xf86RandR12.c:1764)
==30042== by 0x193967: xf86Wakeup (xf86Events.c:527)
==30042== by 0x15A7FA: WakeupHandler (dixutils.c:428)
==30042== by 0x28DDA5: WaitForSomething (WaitFor.c:235)
==30042== by 0x156601: Dispatch (dispatch.c:366)
==30042== Address 0x98bebf1 is 1 bytes inside a block of size 9 free'd
==30042== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30042== by 0x1AAA51: xf86DeleteMode (xf86Mode.c:1985)
==30042== by 0x1C7617: xf86ProbeOutputModes (xf86Crtc.c:1575)
==30042== by 0x1D06B3: xf86RandR12GetInfo12 (xf86RandR12.c:1538)
==30042== by 0x20285C: RRGetInfo (rrinfo.c:202)
==30042== by 0x20743D: rrGetScreenResources (rrscreen.c:337)
==30042== by 0x1568B0: Dispatch (dispatch.c:442)
However, if you look you can find the problem happen a lot earlier,
if you add free(mode->name[1]); to get where the allocation occured:
==30042== Invalid free() / delete / delete[] / realloc()
==30042== at 0x4C2A82E: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30042== by 0x1AAA48: xf86DeleteMode (xf86Mode.c:1984)
==30042== by 0x1C7617: xf86ProbeOutputModes (xf86Crtc.c:1575)
==30042== by 0x1D06B3: xf86RandR12GetInfo12 (xf86RandR12.c:1538)
==30042== by 0x20285C: RRGetInfo (rrinfo.c:202)
==30042== by 0x20743D: rrGetScreenResources (rrscreen.c:337)
==30042== by 0x1568B0: Dispatch (dispatch.c:442)
==30042== by 0x1456D9: main (main.c:288)
==30042== Address 0x98bebf1 is 1 bytes inside a block of size 9 alloc'd
==30042== at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30042== by 0x296638: XNFalloc (utils.c:1048)
==30042== by 0x8CBA432: RADEONFPNativeMode (radeon_modes.c:148)
==30042== by 0x8CBB333: RADEONProbeOutputModes (radeon_modes.c:528)
==30042== by 0x8CB6336: radeon_get_modes (radeon_output.c:1297)
==30042== by 0x1C7706: xf86ProbeOutputModes (xf86Crtc.c:1614)
==30042== by 0x1C9334: xf86InitialConfiguration (xf86Crtc.c:2399)
==30042== by 0x8C4DFE3: RADEONPreInit (radeon_driver.c:3203)
==30042== by 0x197137: InitOutput (xf86Init.c:599)
==30042== by 0x14550C: main (main.c:205)
Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
---
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 2c8878f..8531b7b 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -362,6 +362,8 @@ xf86CrtcSetModeTransform(xf86CrtcPtr crtc, DisplayModePtr mode,
done:
if (ret) {
+ crtc->mode.name = strdup(mode->name);
+ free(saved_mode.name);
crtc->active = TRUE;
if (scrn->pScreen)
xf86CrtcSetScreenSubpixelOrder(scrn->pScreen);
@@ -2417,6 +2419,7 @@ xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow)
xf86CrtcPtr crtc = config->crtc[c];
crtc->enabled = FALSE;
+ free(&crtc->desiredMode.name);
memset(&crtc->desiredMode, '\0', sizeof(crtc->desiredMode));
/* Set default gamma for all crtc's. */
/* This is done to avoid problems later on with cloned outputs. */
@@ -2437,6 +2440,7 @@ xf86InitialConfiguration(ScrnInfoPtr scrn, Bool canGrow)
if (mode && crtc) {
crtc->desiredMode = *mode;
+ crtc->desiredMode.name = strdup(mode->name);
crtc->desiredRotation = output->initial_rotation;
crtc->desiredX = output->initial_x;
crtc->desiredY = output->initial_y;
@@ -2613,6 +2617,7 @@ xf86SetDesiredModes(ScrnInfoPtr scrn)
continue;
/* Mark that we'll need to re-set the mode for sure */
+ free(crtc->mode.name);
memset(&crtc->mode, 0, sizeof(crtc->mode));
if (!crtc->desiredMode.CrtcHDisplay) {
DisplayModePtr mode =
@@ -2620,7 +2625,9 @@ xf86SetDesiredModes(ScrnInfoPtr scrn)
if (!mode)
return FALSE;
+ free(crtc->desiredMode.name);
crtc->desiredMode = *mode;
+ crtc->desiredMode.name = strdup(mode->name);
crtc->desiredRotation = RR_Rotate_0;
crtc->desiredTransformPresent = FALSE;
crtc->desiredX = 0;
@@ -2759,7 +2766,9 @@ xf86SetSingleMode(ScrnInfoPtr pScrn, DisplayModePtr desired, Rotation rotation)
if (!xf86CrtcSetModeTransform(crtc, crtc_mode, rotation, NULL, 0, 0))
ok = FALSE;
else {
+ free(crtc->desiredMode.name);
crtc->desiredMode = *crtc_mode;
+ crtc->desiredMode.name = strdup(crtc_mode->name);
crtc->desiredRotation = rotation;
crtc->desiredTransformPresent = FALSE;
crtc->desiredX = 0;
@@ -2854,6 +2863,7 @@ xf86DisableUnusedFunctions(ScrnInfoPtr pScrn)
if (!crtc->enabled) {
crtc->funcs->dpms(crtc, DPMSModeOff);
+ free(crtc->mode.name);
memset(&crtc->mode, 0, sizeof(crtc->mode));
xf86RotateDestroy(crtc);
crtc->active = FALSE;
diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index 59b6f82..48cef30 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1216,7 +1216,9 @@ xf86RandR12CrtcSet(ScreenPtr pScreen,
/*
* Save the last successful setting for EnterVT
*/
+ free(crtc->desiredMode.name);
crtc->desiredMode = mode;
+ crtc->desiredMode.name = strdup(mode.name);
crtc->desiredRotation = rotation;
if (transform) {
crtc->desiredTransform = *transform;
More information about the xorg-devel
mailing list