Re: [hackers] [dwm][PATCH 1/2] don't iterate through monitors in cleanupmon

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Mon, 27 Jan 2020 15:37:37 +0100

On Mon, Jan 27, 2020 at 07:55:57AM -0600, Devin J. Pohly wrote:
> Since both updategeom and cleanup are already iterating over the mons
> list, it is wasteful to re-iterate the list in their calls to
> cleanupmon. Instead, make the caller of cleanupmon responsible for
> removing the monitor from mons.
>
> This uses the pointer-to-pointer technique from detach for iterating and
> removing monitors.
> ---
> dwm.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/dwm.c b/dwm.c
> index 4465af1..958a0e4 100644
> --- a/dwm.c
> +++ b/dwm.c
> _AT_@ -476,12 +476,13 @@ cleanup(void)
>
> view(&a);
> selmon->lt[selmon->sellt] = &foo;
> - for (m = mons; m; m = m->next)
> + while ((m = mons)) {
> while (m->stack)
> unmanage(m->stack, 0);
> + mons = mons->next;
> + cleanupmon(m);
> + }
> XUngrabKey(dpy, AnyKey, AnyModifier, root);
> - while (mons)
> - cleanupmon(mons);
> for (i = 0; i < CurLast; i++)
> drw_cur_free(drw, cursor[i]);
> for (i = 0; i < LENGTH(colors); i++)
> _AT_@ -496,14 +497,6 @@ cleanup(void)
> void
> cleanupmon(Monitor *mon)
> {
> - Monitor *m;
> -
> - if (mon == mons)
> - mons = mons->next;
> - else {
> - for (m = mons; m && m->next != mon; m = m->next);
> - m->next = mon->next;
> - }
> XUnmapWindow(dpy, mon->barwin);
> XDestroyWindow(dpy, mon->barwin);
> free(mon);
> _AT_@ -1856,7 +1849,7 @@ updategeom(void)
> if (XineramaIsActive(dpy)) {
> int i, j, n, nn;
> Client *c;
> - Monitor *m;
> + Monitor *m, **pm;
> XineramaScreenInfo *info = XineramaQueryScreens(dpy, &nn);
> XineramaScreenInfo *unique = NULL;
>
> _AT_@ -1890,8 +1883,9 @@ updategeom(void)
> updatebarpos(m);
> }
> } else { /* less monitors available nn < n */
> - for (i = nn; i < n; i++) {
> - for (m = mons; m && m->next; m = m->next);
> + for (i = 0, pm = &mons; i < nn; i++, pm = &(*pm)->next);
> + for (/* i */; i < n; i++) {
> + m = *pm;
> while ((c = m->clients)) {
> dirty = 1;
> m->clients = c->next;
> _AT_@ -1902,6 +1896,7 @@ updategeom(void)
> }
> if (m == selmon)
> selmon = mons;
> + *pm = (*pm)->next;
> cleanupmon(m);
> }
> }
> --
> 2.25.0
>
>

No, just stop.

This is not "wasteful". It is readable.

-- 
Kind regards,
Hiltjo
Received on Mon Jan 27 2020 - 15:37:37 CET

This archive was generated by hypermail 2.3.0 : Mon Jan 27 2020 - 15:48:32 CET