On Mon, Jan 27, 2020 at 03:37:37PM +0100, Hiltjo Posthuma wrote:
> 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
> > @@ -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++)
> > @@ -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);
> > @@ -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;
> >
> > @@ -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;
> > @@ -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.
>
Thanks for the feedback. I still think the current implementation is
inefficient, but I agree that hackable code should be prioritized.
I will keep thinking. Perhaps there is a different approach that can
improve both simplicity and understandability. :)
*dp
--
<><
