Confusing or confused code

[Adam Jackson]

On Thu, 2010-03-11 at 12:59 +0100, Michael Thayer wrote:
> Hello,
> 
> I suspect that this code:
> 
> http://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/common/xf86Mode.c?id=326429badfc76885e4652ddc72860810c0e8d102#n1300
> 
> until line 1313 is supposed to be freeing the old list at
> scrp->clockRanges, duplicating the user supplied list and storing it
> there.  If that is right, then it looks to me though like it will not
> work unless scrp->clockRanges is initially NULL

AFAICT yes; if ->clockRanges is not initially NULL, then we will not
store the duplicated list there.  Which seems goofy.

> and the user only supplies a list with a single element.

In that, after the first memcpy, storeClockRanges->next and cp->next
will point to the same place, so subsequent copies will just be lost in
space, yeah.

> Did I miss something important there?

I think your analysis is right, that code's garbage.

At least in the case of virtual drivers like vboxvideo, I think we
should reasonably allow you to just pass in NULL for clockRanges.  But
it's trivial to set up just one range that's sufficient to cover
everything.  Gross, but sufficient.

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100315/83458afb/attachment.pgp>