[PATCH xserver] edid: Prune duplicates after adding modes from DDC

[Michel Dänzer]

On 24/02/17 05:21 AM, Emil Velikov wrote:
> On Wednesday, 22 February 2017, Michel Dänzer <michel at daenzer.net
> <mailto:michel at daenzer.net>> wrote:
> 
>     @@ -1198,21 +1198,19 @@ xf86EdidMonitorSet(int scrnIndex, MonPtr
>     Monitor, xf86MonPtr DDC)
>              if (!Monitor->nHsync || !Monitor->nVrefresh)
>                  DDCGuessRangesFromModes(scrnIndex, Monitor, Modes);
> 
>     -        /* look for last Mode */
>     -        Mode = Modes;
>     -
>     -        while (Mode->next)
>     -            Mode = Mode->next;
>     -
>              /* add to MonPtr */
>              if (Monitor->Modes) {
>                  Monitor->Last->next = Modes;
>                  Modes->prev = Monitor->Last;
>     -            Monitor->Last = Mode;
>              }
>              else {
>                  Monitor->Modes = Modes;
>     -            Monitor->Last = Mode;
>              }
>     +
>     +        xf86PruneDuplicateModes(Monitor->Modes);
>     +
> 
> Wouldn't it be better to add the mode only if it doesn't exist, or it
> doesn't really matter ?

Modes points to a list of modes, not just a single mode.
xf86PruneDuplicateModes is supposed to eliminate duplicates from the
combined list of modes.


>     +        /* Update pointer to last mode */
>     +        for (Mode = Monitor->Modes; Mode && Mode->next; Mode =
>     Mode->next);
>     +        Monitor->Last = Mode;
> 
> Perhaps I'm just tired but this looks like this will overwrite
> Monitor->Last multiple times, right?

I guess you missed the semicolon after the for statement. I'll change it
to {} for the next revision.


> P.s. Sending from my phone - pardon the html formatting. 

It could have waited until you got access to a decent mailer? :)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer