Make xf86ValidateModes actually copy clock range list to screen pointer

Our in-house parfait 1.1 code analysis tool complained that every exit
path from xf86ValidateModes() in hw/xfree86/common/xf86Mode.c leaks the
storeClockRanges allocation made at line 1501 with XNFalloc.

Investigating, it seems that this code to copy the clock range list to
the clockRanges list in the screen pointer is just plain insane, and
according to git, has been since we first imported it from XFree86.

We start at line 1495 by walking the linked list from scrp->clockRanges
until we find the end.  But that was just a diversion, since we've found
the end and immediately forgotten it, and thus at 1499 we know that
storeClockRanges is NULL, but that's not a problem since we're going to
immediately overwrite that value as the first thing in the loop.

So we move on through this loop at 1499, which takes us through the
linked list from the clockRanges variable, and for every entry in
that list allocates a new structure and copies cp to it.  If we've
not filled in the screen's clockRanges pointer yet, we set it to
the first storeClockRanges we copied from cp.   Otherwise, as best
I can tell, we just drop it into memory and let it leak away, as
parfait warned.

And then we hit the loop action, which if we haven't hit the end of
the cp list, advances cp to the next item in the list, and then just
for the fun of it, also sets storeClockRanges to the ->next pointer it
has just copied from cp as well, even though it's going to overwrite
it as the very first instruction in the loop body.

v2: rewritten using nt_list_* macros from Xorg's list.h header

Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/hw/xfree86/common/xf86Mode.c b/hw/xfree86/common/xf86Mode.c
index d80dec8..706ab64 100644 (file)
--- a/hw/xfree86/common/xf86Mode.c
+++ b/hw/xfree86/common/xf86Mode.c
@@ -1370,7 +1370,6 @@ xf86ValidateModes(ScrnInfoPtr scrp, DisplayModePtr availModes,
     int saveType;
     PixmapFormatRec *BankFormat;
     ClockRangePtr cp;
-    ClockRangePtr storeClockRanges;
     int numTimings = 0;
     range hsync[MAX_HSYNC];
     range vrefresh[MAX_VREFRESH];
@@ -1492,16 +1491,14 @@ xf86ValidateModes(ScrnInfoPtr scrp, DisplayModePtr availModes,
     /*
      * Store the clockRanges for later use by the VidMode extension.
      */
-    storeClockRanges = scrp->clockRanges;
-    while (storeClockRanges != NULL) {
-        storeClockRanges = storeClockRanges->next;
-    }
-    for (cp = clockRanges; cp != NULL; cp = cp->next,
-         storeClockRanges = storeClockRanges->next) {
-        storeClockRanges = xnfalloc(sizeof(ClockRange));
+    nt_list_for_each_entry(cp, clockRanges, next) {
+        ClockRangePtr newCR = xnfalloc(sizeof(ClockRange));
+        memcpy(newCR, cp, sizeof(ClockRange));
+        newCR->next = NULL;
         if (scrp->clockRanges == NULL)
-            scrp->clockRanges = storeClockRanges;
-        memcpy(storeClockRanges, cp, sizeof(ClockRange));
+            scrp->clockRanges = newCR;
+        else
+            nt_list_append(newCR, scrp->clockRanges, ClockRange, next);
     }
 
     /* Determine which pixmap format to pass to scanLineWidth() */