On 24/02/17 05:21 AM, Emil Velikov wrote:
> On Wednesday, 22 February 2017, Michel Dänzer <mic...@daenzer.net
> <mailto:mic...@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
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
