comment updates

src/config.c

@@ -258,12 +258,14 @@ merge_ops( int cops, int ops[] )
 
 	aops = ops[F] | ops[N];
 	if (ops[F] & XOP_HAVE_TYPE) {
-		if (aops & OP_MASK_TYPE) {
-			if (aops & cops & OP_MASK_TYPE) {
+		if (aops & OP_MASK_TYPE) {  // PullNew, etc.
+			if (aops & cops & OP_MASK_TYPE) {  // Overlapping New, etc.
 			  cfl:
 				error( "Conflicting Sync args specified.\n" );
 				return 1;
 			}
+			// Mix in non-overlapping Push/Pull or New, etc.
+			// Do the ops first, so e.g. PullNew Push Flags will error out.
 			ops[F] |= cops & OP_MASK_TYPE;
 			ops[N] |= cops & OP_MASK_TYPE;
 			if (cops & XOP_PULL) {
@@ -276,7 +278,7 @@ merge_ops( int cops, int ops[] )
 					goto cfl;
 				ops[F] |= OP_MASK_TYPE;
 			}
-		} else if (cops & (OP_MASK_TYPE|XOP_MASK_DIR)) {
+		} else if (cops & (OP_MASK_TYPE | XOP_MASK_DIR)) {  // Pull New, etc.
 			if (!(cops & OP_MASK_TYPE))
 				cops |= OP_MASK_TYPE;
 			else if (!(cops & XOP_MASK_DIR))

src/driver.h

@@ -107,7 +107,7 @@ typedef struct {
 #define DRV_BOX_BAD     2
 /* Failed to connect store. */
 #define DRV_STORE_BAD   3
-/* The command has been cancel()ed or cancel_store()d. */
+/* The command has been cancel_cmds()d or cancel_store()d. */
 #define DRV_CANCELED    4
 
 /* All memory belongs to the driver's user, unless stated otherwise. */
@@ -241,8 +241,8 @@ struct driver {
 
 	/* Add/remove the named flags to/from the given message. The message may be either
 	 * a pre-fetched one (in which case the in-memory representation is updated),
-	 * or it may be identifed by UID only. The operation may be delayed until commit()
-	 * is called. */
+	 * or it may be identifed by UID only.
+	 * The operation may be delayed until commit_cmds() is called. */
 	void (*set_msg_flags)( store_t *ctx, message_t *msg, uint uid, int add, int del,
 	                       void (*cb)( int sts, void *aux ), void *aux );
 

src/drv_imap.c

@@ -135,7 +135,7 @@ union imap_store {
 		enum { GreetingPending = 0, GreetingBad, GreetingOk, GreetingPreauth } greeting;
 		int expectBYE;  // LOGOUT is in progress
 		int expectEOF;  // received LOGOUT's OK or unsolicited BYE
-		int canceling;  // imap_cancel() is in progress
+		int canceling;  // imap_cancel_cmds() is in progress
 		union {
 			void (*imap_open)( int sts, void *aux );
 			void (*imap_cancel)( void *aux );
@@ -1712,7 +1712,7 @@ imap_socket_read( void *aux )
 						continue;
 					}
 					resp = RESP_NO;
-					if (cmdp->param.failok)
+					if (cmdp->param.failok)  // SELECT
 						goto doresp;
 				} else /*if (!strcmp( "BAD", arg ))*/
 					resp = RESP_CANCEL;
@@ -3164,6 +3164,7 @@ imap_close_box( store_t *gctx,
 	} else {
 		/* This is inherently racy: it may cause messages which other clients
 		 * marked as deleted to be expunged without being trashed. */
+		// Note that, to save bandwidth, we don't use EXPUNGE.
 		imap_cmd_simple_t *cmd;
 		INIT_IMAP_CMD(imap_cmd_simple_t, cmd, cb, aux)
 		imap_exec( ctx, &cmd->gen, imap_done_simple_box, "CLOSE" );
@@ -3289,7 +3290,7 @@ imap_find_new_msgs( store_t *gctx, uint newuid,
 	INIT_IMAP_CMD(imap_cmd_find_new_t, cmd, cb, aux)
 	cmd->out_msgs = ctx->msgapp;
 	cmd->uid = newuid;
-	// Some servers fail to enumerate recently STOREd messages without syncing first.
+	// Some servers fail to enumerate recently APPENDed messages without syncing first.
 	imap_exec( ctx, &cmd->gen, imap_find_new_msgs_p2, "CHECK" );
 }
 

src/sync.c

@@ -573,7 +573,7 @@ msg_fetched( int sts, void *aux )
 	case DRV_MSG_BAD:
 		vars->cb( SYNC_NOGOOD, 0, vars );
 		break;
-	default:
+	default:  // DRV_BOX_BAD
 		vars->cb( SYNC_FAIL, 0, vars );
 		break;
 	}
@@ -599,7 +599,7 @@ msg_stored( int sts, uint uid, void *aux )
 		      str_fn[t], vars->msg->uid, str_fn[1-t] );
 		vars->cb( SYNC_NOGOOD, 0, vars );
 		break;
-	default:
+	default:  // DRV_BOX_BAD
 		vars->cb( SYNC_FAIL, 0, vars );
 		break;
 	}
@@ -1094,7 +1094,7 @@ load_state( sync_vars_t *svars )
 					case '*':
 						debug( "flags now %u\n", t3 );
 						srec->flags = (uchar)t3;
-						srec->aflags[F] = srec->aflags[N] = 0;
+						srec->aflags[F] = srec->aflags[N] = 0;  // Clear F_DELETED from purge
 						srec->wstate &= ~W_PURGE;
 						break;
 					case '~':
@@ -1413,6 +1413,11 @@ box_opened2( sync_vars_t *svars, int t )
 				opts[t] |= OPEN_NEW|OPEN_FLAGS;
 		}
 	}
+	// While only new messages can cause expiration due to displacement,
+	// updating flags can cause expiration of already overdue messages.
+	// The latter would also apply when the expired box is the source,
+	// but it's more natural to treat it as read-only in that case.
+	// OP_RENEW makes sense only for legacy S_SKIPPED entries.
 	if ((chan->ops[N] & (OP_NEW|OP_RENEW|OP_FLAGS)) && chan->max_messages)
 		opts[N] |= OPEN_OLD|OPEN_NEW|OPEN_FLAGS;
 	if (svars->replayed)
@@ -1769,7 +1774,7 @@ box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux
 					any_new[t] = 1;
 				}
 			}
-			srec = nsrec;
+			srec = nsrec;  // Minor optimization: skip freshly created placeholder entry.
 		}
 	}
 
@@ -1987,7 +1992,8 @@ box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux
 			} else {
 				/* The trigger is an expiration transaction being ongoing ... */
 				if ((t == N) && ((shifted_bit(srec->status, S_EXPIRE, S_EXPIRED) ^ srec->status) & S_EXPIRED)) {
-					/* ... but the actual action derives from the wanted state. */
+					// ... but the actual action derives from the wanted state -
+					// so that canceled transactions are rolled back as well.
 					if (srec->wstate & W_NEXPIRE)
 						aflags |= F_DELETED;
 					else
@@ -2377,7 +2383,7 @@ box_closed_p2( sync_vars_t *svars, int t )
 	for (t = 0; t < 2; t++) {
 		// Committing maxuid is delayed until all messages were propagated, to
 		// ensure that all pending messages are still loaded next time in case
-		// of interruption - in particular skipping big messages would otherwise
+		// of interruption - in particular skipping messages would otherwise
 		// up the limit too early.
 		if (svars->maxuid[t] != svars->oldmaxuid[t])
 			JLOG( "N %d %u", (t, svars->maxuid[t]), "up maxuid of %s", str_fn[t] );

src/sync.h

@@ -24,7 +24,7 @@
 #define XOP_PUSH           (1<<8)
 #define XOP_PULL           (1<<9)
 #define  XOP_MASK_DIR      (XOP_PUSH|XOP_PULL)
-#define XOP_HAVE_TYPE      (1<<10)
+#define XOP_HAVE_TYPE      (1<<10)  // Aka mode; at least one of dir and type
 // The following must all have the same bit shift from the corresponding OP_* flags.
 #define XOP_HAVE_EXPUNGE   (1<<11)
 #define XOP_HAVE_CREATE    (1<<12)