[PATCH] fix add default mode

[Hong Liu]

On Fri, 2008-03-07 at 09:27 -0500, Adam Jackson wrote:
> On Fri, 2008-03-07 at 12:47 +0100, Matthias Hopf wrote:
> > On Mar 07, 08 14:31:04 +0800, Hong Liu wrote:
> > > The build-in Modes don't have a name now
> > > (after Adam's commit e65e51a99b17a0510782775f010e9820ca567fcb),
> > > this may cause problem for code which uses the (name != NULL)
> > > as a condition for termination of the xf86DefaultModes array.
> 
> Sigh.  I thought I'd fixed the users of xf86DefaultModes to invoke
> xf86DuplicateMode for this exact reason.  Apparently not.
> 
> > Uh-oh. IMHO it is a bug that modes w/o a name exist. So why on earth has
> > this commit been made in the first place? If it's just that the data
> > table should be able to be put into the text segment, a (const char *)
> > casting of the strings should suffice...
> 
> That doesn't work.  The string will be emitted as a constant, yes, but
> the structure will be filled in with a pointer to that string.  You
> can't resolve that pointer until runtime, because you don't know where
> you're going to be loaded in the address space, which means the whole
> table has to be writable so ld.so can write the relocation in.
> 
> The right thing is to use sizeof(xf86DefaultModes) /
> sizeof(DisplayModeRec) as the size of the list instead.  I've pushed a
> change that should fix this (and also makes the consumers of
> xf86DefaultModes pleasantly simpler).

There is still a small problem in addDefaultModes. The modeIsPresent
only checks name of the mode, so mode with same name but different
refresh rates in xf86DefaultModes will not be added.

So you may need to use xf86ModesEqual to do the check.

> 
> - ajax
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg