Mail: separate deletion from relinking messages

Mutating lists that are being iterated is needlessly error
prone, and we were removing the wrong message in some cases
if it the dummy got inserted in the right place.

Separating deletion into a redraw/relink and zap phase
simplifies the problem.

diff --git a/sys/src/cmd/upas/Mail/mail.h b/sys/src/cmd/upas/Mail/mail.h
index a6604d1043c4af12d1ddf7a34069b1b43272ad81..f9cc727a8fcc15cabf7f710a98494208a6d1ba67 100644 (file)
--- a/sys/src/cmd/upas/Mail/mail.h
+++ b/sys/src/cmd/upas/Mail/mail.h
@@ -15,6 +15,7 @@ enum {
        Sdummy  = 1<<0, /* message placeholder */
        Stoplev = 1<<1, /* not a response to anything */
        Sopen   = 1<<2, /* opened for viewing */
+       Szap    = 1<<3, /* flushed, to be removed from list */
 };
 
 enum {

diff --git a/sys/src/cmd/upas/Mail/mbox.c b/sys/src/cmd/upas/Mail/mbox.c
index 5b05dc52913b0bc2f183fc4c99c4186819d344cf..6d486456b9acf092c5cb97e20fd11c91e50a438f 100644 (file)
--- a/sys/src/cmd/upas/Mail/mbox.c
+++ b/sys/src/cmd/upas/Mail/mbox.c
@@ -144,7 +144,7 @@ mesglineno(Mesg *msg, int *depth)
                                break;
                        o += p->child[i]->nsub + 1;
                }
-               if(!(p->state & Sdummy)){
+               if(!(p->state & (Sdummy|Szap))){
                        o++;
                        d++;
                }
@@ -157,7 +157,7 @@ mesglineno(Mesg *msg, int *depth)
                        break;
                if(m->state & Stoplev){
                        n += mbox.mesg[i]->nsub;
-                       if(!(m->state & Sdummy))
+                       if(!(m->state & (Sdummy|Szap)))
                                n++;
                }
 
@@ -679,38 +679,43 @@ mbflush(char **, int)
        char *path;
        Mesg *m, *p;
 
-       i = 0;
        path = estrjoin(maildir, "/ctl", nil);
        fd = open(path, OWRITE);
        free(path);
        if(fd == -1)
                sysfatal("open mbox: %r");
-       while(i < mbox.nmesg){
+       for(i = 0; i < mbox.nmesg; i++){
                m = mbox.mesg[i];
-               if((m->state & Sopen) || !(m->flags & (Fdel|Ftodel))){
-                       i++;
+               p = m->parent;
+               if(m->state & (Sopen|Szap) || (m->flags & (Fdel|Ftodel)) == 0)
                        continue;
-               }
+
                ln = mesglineno(m, nil);
                fprint(mbox.addr, "%d,%d", ln, ln+m->nsub);
                write(mbox.data, "", 0);
                if(m->flags & Ftodel)
                        fprint(fd, "delete %s %d", mailbox, atoi(m->name));
 
-               p = m->parent;
                removeid(m);
+               m->state |= Szap;
                if(p == nil && m->nsub != 0){
                        p = placeholder(m->messageid, m->time, 1);
                        p->nsub = m->nsub + 1;
-                       mbox.mesg[i] = p;
                }
                if(p != nil)
                        relinkmsg(p, m);
                for(j = 0; j < m->nchild; j++)
                        mbredraw(m->child[j], 1, 1);
-               memmove(&mbox.mesg[i], &mbox.mesg[i+1], (mbox.nmesg - i)*sizeof(Mesg*));
-               mbox.nmesg--;
        }
+
+       for(i = 0, j = 0; i < mbox.nmesg; i++){
+               m = mbox.mesg[i];
+               if((m->state & Szap) != 0)
+                       mesgfree(m);
+               else
+                       mbox.mesg[j++] = m;
+       }
+               
        close(fd);
        fprint(mbox.ctl, "clean\n");
 }