Re: [9front] [patch] upas: plumb flag updates on messages.

[cinap_lenrek@felloff.net]

ok, small remarks.

 char*
 syncmbox(Mailbox *mb, int doplumb)
 {
@@ -90,16 +87,14 @@
 				cachehash(mb, m);
 				m->cstate |= Cnew;
 				n++;
-			} else if(!doplumb)
-				m->cstate &= ~Cnew;
-			if(m->cstate & Cnew){
-				if(ensurecache(mb, m) == 0){
-					if(doplumb)
-						mailplumb(mb, m);
+			}
+			if(doplumb){
+				if((m->cstate & Cnew) && ensurecache(mb, m) == 0)
 					msgdecref(mb, m);
-				}
-				m->cstate &= ~Cnew;
+				if(m->cstate & (Cnew|Cmod))
+					mailplumb(mb, m);
 			}
+			m->cstate &= ~(Cnew|Cmod);
 		}
 		if(m->cstate & Cidxstale)

i think you want to do the msgdecref() of the message *after* mailplumb(),
not before. otherwise its like free before use. i know with the current
code it will still work but it is semantially wrong. if we later change
how cache eviction works that will blow up.

diff -r 7f3f8606fc12 sys/src/cmd/upas/fs/fs.c
--- a/sys/src/cmd/upas/fs/fs.c	Sun Dec 08 11:58:52 2019 -0800
+++ b/sys/src/cmd/upas/fs/fs.c	Sun Dec 08 11:59:39 2019 -0800
@@ -337,14 +337,15 @@
 		mboxfile = mbox;
 	}
 
-	if(mboxfile != nil)
-		if(err = newmbox(mboxfile, "mbox", 0, nil))
-			sysfatal("opening %s: %s", mboxfile, err);
 
 	switch(rfork(RFFDG|RFPROC|RFNAMEG|RFNOTEG|RFREND)){
 	case -1:
 		error("fork");
 	case 0:
+		print("child\n");
+		if(mboxfile != nil)
+			if(err = newmbox(mboxfile, "mbox", 0, nil))
+				sysfatal("opening %s: %s", mboxfile, err);
 		henter(PATH(0, Qtop), dirtab[Qctl],
 			(Qid){PATH(0, Qctl), 0, QTFILE}, nil, nil);
 		close(p[1]);

theres a debug print left there and this changes some semantics.
i know this is because initial sync from imap takes forever. but
people sometimes want to specify mailbox path at the initial
command line and expect error status if the initial sync failed
(like if mounting a mbox file in a script to parse a message).

i think a good compromise would be to add a commandline flag to
specify if you want initial sync in the background or not
(can then be added to /rc/bin/startupasfs).

if we do background syncing, then even if the initial sync
fails, it should try to continue. imagine your imap server
is currently down or network is unavailable, you want it to
continue trying in that case. and sysfataling here makes no
sense as nobody will be listening.

--
cinap