[PATCH] VidMode fixes [was: bug in XF86VidModeAddModeLine?]

[Miguel Freitas]

Hi,

May someone with cvs access check and commit this patch?
(patch for XFree86 4.3.0)

It fixes:

- memory leaks at ProcXF86VidModeModModeLine and
ProcXF86VidModeValidateModeLine

- uninitialized fields of mode structure at ProcXF86VidModeAddModeLine,
VidModeCreateMode and VidModeAddModeline (the last one caused a segfault
trying to delete the new mode)

note: XF86VidModeAddModeLine is _broken_ on current XFree86 releases.


On Thu, 2003-03-13 at 08:03, Billy Biggs wrote:
>   I think this is the patch, but it's surprisingly small and silly, and
> I think it's a patch for X 4.1.  If you ignore the ErrorF()'s I added,
> it's really only three lines of code:  I actually use the requested dot
> clock, I always indicate that modes are OK, and I don't remember what
> the VScan thing does ;-)


VScan is used to divide the refresh rate at xf86CheckModeForMonitor, if
we don't set it the addmodeline is likely to fail. however you should
not set it at VidModeAddModeline because at that point we have already
passed monitor and driver tests.

regards,

Miguel


--- xc/programs/Xserver/Xext/xf86vmode.c        2002-12-21 16:46:51.000000000 -0800
+++ xc-mf/programs/Xserver/Xext/xf86vmode.c     2003-03-13 16:48:42.000000000 -0800
@@ -742,6 +742,7 @@
     if (mode == NULL)
        return BadValue;
 
+    VidModeSetModeValue(mode, VIDMODE_CLOCK, stuff->dotclock);
     VidModeSetModeValue(mode, VIDMODE_H_DISPLAY, stuff->hdisplay);
     VidModeSetModeValue(mode, VIDMODE_H_SYNCSTART, stuff->hsyncstart); 
     VidModeSetModeValue(mode, VIDMODE_H_SYNCEND, stuff->hsyncend);
@@ -989,18 +990,23 @@
            break;
        case MODE_HSYNC:
        case MODE_H_ILLEGAL:
+           xfree(modetmp);
            return VidModeErrorBase + XF86VidModeBadHTimings;
        case MODE_VSYNC:
        case MODE_V_ILLEGAL:
+           xfree(modetmp);
            return VidModeErrorBase + XF86VidModeBadVTimings;
        default:
+           xfree(modetmp);
            return VidModeErrorBase + XF86VidModeModeUnsuitable;
     }
 
     /* Check that the driver is happy with the mode */
     if (VidModeCheckModeForDriver(stuff->screen, modetmp) != MODE_OK) {
+       xfree(modetmp);
        return VidModeErrorBase + XF86VidModeModeUnsuitable;
     }
+    xfree(modetmp);
 
     VidModeSetModeValue(mode, VIDMODE_H_DISPLAY, stuff->hdisplay);
     VidModeSetModeValue(mode, VIDMODE_H_SYNCSTART, stuff->hsyncstart); 
@@ -1029,7 +1035,7 @@
                (xXF86OldVidModeValidateModeLineReq *)client->requestBuffer;
     xXF86VidModeValidateModeLineReq newstuff;
     xXF86VidModeValidateModeLineReply rep;
-    pointer mode, modetmp;
+    pointer mode, modetmp = NULL;
     int len, status, dotClock;
     int ver;
 
@@ -1119,6 +1125,9 @@
     status = VidModeCheckModeForDriver(stuff->screen, modetmp);
 
 status_reply:
+    if(modetmp)
+      xfree(modetmp);
+
     rep.type = X_Reply;
     rep.length = (SIZEOF(xXF86VidModeValidateModeLineReply)
                         - SIZEOF(xGenericReply)) >> 2;
--- xc/programs/Xserver/hw/xfree86/common/xf86VidMode.c 2003-01-28 12:52:28.000000000 
-0800
+++ xc-mf/programs/Xserver/hw/xfree86/common/xf86VidMode.c      2003-03-13 
16:48:18.000000000 -0800
@@ -440,10 +440,13 @@
 
     pScrn = xf86Screens[scrnIndex];
 
+    ((DisplayModePtr)mode)->name         = strdup(""); /* freed by deletemode */
+    ((DisplayModePtr)mode)->status       = MODE_OK;
     ((DisplayModePtr)mode)->next         = pScrn->modes->next;
     ((DisplayModePtr)mode)->prev         = pScrn->modes;
     pScrn->modes->next                   = (DisplayModePtr)mode;
-    ((DisplayModePtr)mode)->next->prev   = (DisplayModePtr)mode;
+    if( ((DisplayModePtr)mode)->next != NULL )
+      ((DisplayModePtr)mode)->next->prev   = (DisplayModePtr)mode;
 
     return TRUE;
 }
@@ -547,6 +550,7 @@
     mode = xalloc(sizeof(DisplayModeRec));
     if (mode != NULL) {
        mode->name          = "";
+       mode->VScan         = 1;    /* divides refresh rate. default = 1 */
        mode->Private       = NULL;
        mode->next          = mode;
        mode->prev          = mode;