From 9162533526a8e92a3b111ef2e63f87ae23ad8134 Mon Sep 17 00:00:00 2001
From: Ori Bernstein <ori@eigenstate.org>
Date: Wed, 10 Mar 2021 16:49:17 -0800
Subject: [PATCH] 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.
---
 sys/src/cmd/upas/Mail/mail.h |  1 +
 sys/src/cmd/upas/Mail/mbox.c | 27 ++++++++++++++++-----------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/sys/src/cmd/upas/Mail/mail.h b/sys/src/cmd/upas/Mail/mail.h
index a6604d104..f9cc727a8 100644
--- 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 5b05dc529..6d486456b 100644
--- 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");
 }