replace DRV_STORE_BAD with a separate bad_callback()

that way we don't have to piggy-back (possibly asynchronous) fatal
errors to particular commands.

internally, the drivers still use synchronous return values as well,
so they don't try to access the invalidated store after calling back.

src/drv_imap.c

@@ -172,12 +172,13 @@ static const char *cap_list[] = {
 #endif
 };
 
-#define RESP_OK    0
-#define RESP_NO    1
-#define RESP_BAD   2
+#define RESP_OK       0
+#define RESP_NO       1
+#define RESP_CANCEL   2
 
 static int get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd );
 
+static void imap_invoke_bad_callback( imap_store_t *ctx );
 
 static const char *Flags[] = {
 	"Draft",
@@ -503,8 +504,8 @@ v_submit_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd,
 	char buf[1024];
 
 	while (ctx->literal_pending)
-		if (get_cmd_result( ctx, 0 ) == RESP_BAD)
-			goto bail;
+		if (get_cmd_result( ctx, 0 ) == RESP_CANCEL)
+			goto bail2;
 
 	if (!cmd)
 		cmd = new_imap_cmd();
@@ -549,6 +550,8 @@ v_submit_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd,
 	return cmd;
 
   bail:
+	imap_invoke_bad_callback( ctx );
+  bail2:
 	free( cmd->param.data );
 	free( cmd->cmd );
 	free( cmd );
@@ -576,7 +579,7 @@ imap_exec( imap_store_t *ctx, struct imap_cmd *cmdp, const char *fmt, ... )
 	cmdp = v_submit_imap_cmd( ctx, cmdp, fmt, ap );
 	va_end( ap );
 	if (!cmdp)
-		return RESP_BAD;
+		return RESP_CANCEL;
 
 	return get_cmd_result( ctx, cmdp );
 }
@@ -590,10 +593,10 @@ imap_exec_b( imap_store_t *ctx, struct imap_cmd *cmdp, const char *fmt, ... )
 	cmdp = v_submit_imap_cmd( ctx, cmdp, fmt, ap );
 	va_end( ap );
 	if (!cmdp)
-		return DRV_STORE_BAD;
+		return DRV_CANCELED;
 
 	switch (get_cmd_result( ctx, cmdp )) {
-	case RESP_BAD: return DRV_STORE_BAD;
+	case RESP_CANCEL: return DRV_CANCELED;
 	case RESP_NO: return DRV_BOX_BAD;
 	default: return DRV_OK;
 	}
@@ -608,10 +611,10 @@ imap_exec_m( imap_store_t *ctx, struct imap_cmd *cmdp, const char *fmt, ... )
 	cmdp = v_submit_imap_cmd( ctx, cmdp, fmt, ap );
 	va_end( ap );
 	if (!cmdp)
-		return DRV_STORE_BAD;
+		return DRV_CANCELED;
 
 	switch (get_cmd_result( ctx, cmdp )) {
-	case RESP_BAD: return DRV_STORE_BAD;
+	case RESP_CANCEL: return DRV_CANCELED;
 	case RESP_NO: return DRV_MSG_BAD;
 	default: return DRV_OK;
 	}
@@ -631,8 +634,8 @@ process_imap_replies( imap_store_t *ctx )
 {
 	while (ctx->num_in_progress > max_in_progress ||
 	       socket_pending( &ctx->buf.sock ))
-		if (get_cmd_result( ctx, 0 ) == RESP_BAD)
-			return RESP_BAD;
+		if (get_cmd_result( ctx, 0 ) == RESP_CANCEL)
+			return RESP_CANCEL;
 	return RESP_OK;
 }
 
@@ -905,7 +908,7 @@ parse_response_code( imap_store_t *ctx, struct imap_cmd *cmd, char *s )
 	s++;
 	if (!(p = strchr( s, ']' ))) {
 		error( "IMAP error: malformed response code\n" );
-		return RESP_BAD;
+		return RESP_CANCEL;
 	}
 	*p++ = 0;
 	arg = next_arg( &s );
@@ -914,12 +917,12 @@ parse_response_code( imap_store_t *ctx, struct imap_cmd *cmd, char *s )
 		    (ctx->gen.uidvalidity = strtoll( arg, &earg, 10 ), *earg))
 		{
 			error( "IMAP error: malformed UIDVALIDITY status\n" );
-			return RESP_BAD;
+			return RESP_CANCEL;
 		}
 	} else if (!strcmp( "UIDNEXT", arg )) {
 		if (!(arg = next_arg( &s )) || (ctx->uidnext = strtol( arg, &p, 10 ), *p)) {
 			error( "IMAP error: malformed NEXTUID status\n" );
-			return RESP_BAD;
+			return RESP_CANCEL;
 		}
 	} else if (!strcmp( "CAPABILITY", arg )) {
 		parse_capability( ctx, s );
@@ -935,7 +938,7 @@ parse_response_code( imap_store_t *ctx, struct imap_cmd *cmd, char *s )
 		    !(arg = next_arg( &s )) || !(*(int *)cmd->param.aux = atoi( arg )))
 		{
 			error( "IMAP error: malformed APPENDUID status\n" );
-			return RESP_BAD;
+			return RESP_CANCEL;
 		}
 	}
 	return RESP_OK;
@@ -1005,14 +1008,14 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 
 	for (;;) {
 		if (buffer_gets( &ctx->buf, &cmd ))
-			return RESP_BAD;
+			break;
 
 		arg = next_arg( &cmd );
 		if (*arg == '*') {
 			arg = next_arg( &cmd );
 			if (!arg) {
 				error( "IMAP error: unable to parse untagged response\n" );
-				return RESP_BAD;
+				break;
 			}
 
 			if (!strcmp( "NAMESPACE", arg )) {
@@ -1035,15 +1038,15 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 					ctx->gen.recent = atoi( arg );
 				else if(!strcmp ( "FETCH", arg1 )) {
 					if (parse_fetch( ctx, cmd ))
-						return RESP_BAD;
+						break; /* stream is likely to be useless now */
 				}
 			} else {
-				error( "IMAP error: unable to parse untagged response\n" );
-				return RESP_BAD;
+				error( "IMAP error: unrecognized untagged response '%s'\n", arg );
+				break; /* this may mean anything, so prefer not to spam the log */
 			}
 		} else if (!ctx->in_progress) {
 			error( "IMAP error: unexpected reply: %s %s\n", arg, cmd ? cmd : "" );
-			return RESP_BAD;
+			break; /* this may mean anything, so prefer not to spam the log */
 		} else if (*arg == '+') {
 			/* This can happen only with the last command underway, as
 			   it enforces a round-trip. */
@@ -1055,16 +1058,16 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 				free( cmdp->param.data );
 				cmdp->param.data = 0;
 				if (n != (int)cmdp->param.data_len)
-					return RESP_BAD;
+					break;
 			} else if (cmdp->param.cont) {
 				if (cmdp->param.cont( ctx, cmdp, cmd ))
-					return RESP_BAD;
+					break;
 			} else {
 				error( "IMAP error: unexpected command continuation request\n" );
-				return RESP_BAD;
+				break;
 			}
 			if (socket_write( &ctx->buf.sock, "\r\n", 2 ) != 2)
-				return RESP_BAD;
+				break;
 			if (!cmdp->param.cont)
 				ctx->literal_pending = 0;
 			if (!tcmd)
@@ -1075,7 +1078,7 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 				if (cmdp->tag == tag)
 					goto gottag;
 			error( "IMAP error: unexpected tag %s\n", arg );
-			return RESP_BAD;
+			break;
 		  gottag:
 			if (!(*pcmdp = cmdp->next))
 				ctx->in_progress_append = pcmdp;
@@ -1092,14 +1095,14 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 					if (cmdp->param.create && cmd && (cmdp->param.trycreate || !memcmp( cmd, "[TRYCREATE]", 11 ))) { /* SELECT, APPEND or UID COPY */
 						p = strchr( cmdp->cmd, '"' );
 						if (!submit_imap_cmd( ctx, 0, "CREATE %.*s", strchr( p + 1, '"' ) - p + 1, p )) {
-							resp = RESP_BAD;
+							resp = RESP_CANCEL;
 							goto normal;
 						}
 						/* not waiting here violates the spec, but a server that does not
 						   grok this nonetheless violates it too. */
 						cmdp->param.create = 0;
 						if (!submit_imap_cmd( ctx, cmdp, 0 )) {
-							resp = RESP_BAD;
+							resp = RESP_CANCEL;
 							goto abnormal;
 						}
 						if (!tcmd)
@@ -1108,13 +1111,15 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 					}
 					resp = RESP_NO;
 				} else /*if (!strcmp( "BAD", arg ))*/
-					resp = RESP_BAD;
+					resp = RESP_CANCEL;
 				error( "IMAP command '%s' returned an error: %s %s\n",
 				       memcmp( cmdp->cmd, "LOGIN", 5 ) ? cmdp->cmd : "LOGIN <user> <pass>",
 				       arg, cmd ? cmd : "" );
 			}
 			if ((resp2 = parse_response_code( ctx, cmdp, cmd )) > resp)
 				resp = resp2;
+			if (resp == RESP_CANCEL)
+				imap_invoke_bad_callback( ctx );
 		  normal:
 			if (cmdp->param.done)
 				cmdp->param.done( ctx, cmdp, resp );
@@ -1126,7 +1131,8 @@ get_cmd_result( imap_store_t *ctx, struct imap_cmd *tcmd )
 				return resp;
 		}
 	}
-	/* not reached */
+	imap_invoke_bad_callback( ctx );
+	return RESP_CANCEL;
 }
 
 static void
@@ -1150,13 +1156,34 @@ imap_cancel_store( store_t *gctx )
 	free( ctx );
 }
 
+static void
+imap_invoke_bad_callback( imap_store_t *ctx )
+{
+	if (ctx->gen.bad_callback)
+		ctx->gen.bad_callback( ctx->gen.bad_callback_aux );
+}
+
 static store_t *unowned;
 
+static void
+imap_cancel_unowned( void *gctx )
+{
+	store_t *store, **storep;
+
+	for (storep = &unowned; (store = *storep); storep = &store->next)
+		if (store == gctx) {
+			*storep = store->next;
+			break;
+		}
+	imap_cancel_store( gctx );
+}
+
 static void
 imap_disown_store( store_t *gctx )
 {
 	free_generic_messages( gctx->msgs );
 	gctx->msgs = 0;
+	set_bad_callback( gctx, imap_cancel_unowned, gctx );
 	gctx->next = unowned;
 	unowned = gctx;
 }
@@ -1181,7 +1208,9 @@ imap_cleanup( void )
 
 	for (ctx = unowned; ctx; ctx = nctx) {
 		nctx = ctx->next;
-		imap_exec( (imap_store_t *)ctx, 0, "LOGOUT" );
+		set_bad_callback( ctx, (void (*)(void *))imap_cancel_store, ctx );
+		if (imap_exec( (imap_store_t *)ctx, 0, "LOGOUT" ) == RESP_CANCEL)
+			continue;
 		imap_cancel_store( ctx );
 	}
 }
@@ -1310,6 +1339,7 @@ imap_open_store( store_conf_t *conf,
 			ctx->gen.boxes = 0;
 			ctx->gen.listed = 0;
 			ctx->gen.conf = conf;
+			set_bad_callback( &ctx->gen, 0, 0 );
 			goto final;
 		}
 
@@ -1618,8 +1648,8 @@ imap_flags_helper( imap_store_t *ctx, int uid, char what, int flags)
 
 	buf[imap_make_flags( flags, buf )] = 0;
 	if (!submit_imap_cmd( ctx, 0, "UID STORE %d %cFLAGS.SILENT %s", uid, what, buf ))
-		return DRV_STORE_BAD;
-	return process_imap_replies( ctx ) == RESP_BAD ? DRV_STORE_BAD : DRV_OK;
+		return DRV_CANCELED;
+	return process_imap_replies( ctx ) == RESP_CANCEL ? DRV_CANCELED : DRV_OK;
 }
 
 static int

src/drv_maildir.c

@@ -159,6 +159,12 @@ maildir_cleanup_drv( void )
 {
 }
 
+static void
+maildir_invoke_bad_callback( store_t *ctx )
+{
+	ctx->bad_callback( ctx->bad_callback_aux );
+}
+
 static void
 maildir_list( store_t *gctx,
               void (*cb)( int sts, void *aux ), void *aux )
@@ -168,7 +174,8 @@ maildir_list( store_t *gctx,
 
 	if (!(dir = opendir( gctx->conf->path ))) {
 		error( "%s: %s\n", gctx->conf->path, strerror(errno) );
-		cb( DRV_STORE_BAD, aux );
+		maildir_invoke_bad_callback( gctx );
+		cb( DRV_CANCELED, aux );
 		return;
 	}
 	while ((de = readdir( dir ))) {
@@ -219,7 +226,7 @@ maildir_free_scan( msglist_t *msglist )
 #define _24_HOURS (3600 * 24)
 
 static int
-maildir_validate( const char *prefix, const char *box, int create )
+maildir_validate( const char *prefix, const char *box, int create, maildir_store_t *ctx )
 {
 	DIR *dirp;
 	struct dirent *entry;
@@ -235,7 +242,8 @@ maildir_validate( const char *prefix, const char *box, int create )
 				if (mkdir( buf, 0700 )) {
 					error( "Maildir error: mkdir %s: %s (errno %d)\n",
 					       buf, strerror(errno), errno );
-					return DRV_STORE_BAD;
+					maildir_invoke_bad_callback( &ctx->gen );
+					return DRV_CANCELED;
 				}
 			  mkdirs:
 				for (i = 0; i < 3; i++) {
@@ -781,8 +789,8 @@ maildir_select( store_t *gctx, int minuid, int maxuid, int *excs, int nexcs,
 	ctx->excs = nfrealloc( excs, nexcs * sizeof(int) );
 	ctx->nexcs = nexcs;
 
-	if (maildir_validate( gctx->path, "", ctx->gen.opts & OPEN_CREATE ) != DRV_OK)
-		return cb( DRV_BOX_BAD, aux );
+	if ((ret = maildir_validate( gctx->path, "", ctx->gen.opts & OPEN_CREATE, ctx )) != DRV_OK)
+		return cb( ret, aux );
 
 	nfsnprintf( uvpath, sizeof(uvpath), "%s/.uidvalidity", gctx->path );
 #ifndef USE_DB
@@ -1007,7 +1015,7 @@ maildir_store_msg( store_t *gctx, msg_data_t *data, int to_trash,
 			free( data->data );
 			return cb( DRV_BOX_BAD, 0, aux );
 		}
-		if ((ret = maildir_validate( gctx->conf->path, gctx->conf->trash, gctx->opts & OPEN_CREATE )) != DRV_OK) {
+		if ((ret = maildir_validate( gctx->conf->path, gctx->conf->trash, gctx->opts & OPEN_CREATE, ctx )) != DRV_OK) {
 			free( data->data );
 			return cb( ret, 0, aux );
 		}
@@ -1147,7 +1155,7 @@ maildir_trash_msg( store_t *gctx, message_t *gmsg,
 		if (!rename( buf, nbuf ))
 			break;
 		if (!stat( buf, &st )) {
-			if ((ret = maildir_validate( gctx->conf->path, gctx->conf->trash, 1 )) != DRV_OK)
+			if ((ret = maildir_validate( gctx->conf->path, gctx->conf->trash, 1, ctx )) != DRV_OK)
 				return cb( ret, aux );
 			if (!rename( buf, nbuf ))
 				break;

src/isync.h

@@ -44,6 +44,12 @@
 # define ATTR_PRINTFLIKE(fmt,var)
 #endif
 
+#ifdef __GNUC__
+# define INLINE __inline__
+#else
+# define INLINE
+#endif
+
 #define EXE "mbsync"
 
 typedef struct {
@@ -148,6 +154,9 @@ typedef struct store {
 	string_list_t *boxes; /* _list results - own */
 	unsigned listed:1; /* was _list already run? */
 
+	void (*bad_callback)( void *aux );
+	void *bad_callback_aux;
+
 	/* currently open mailbox */
 	const char *name; /* foreign! maybe preset? */
 	char *path; /* own */
@@ -159,6 +168,13 @@ typedef struct store {
 	int recent; /* # of recent messages - don't trust this beyond the initial read */
 } store_t;
 
+static INLINE void
+set_bad_callback( store_t *ctx, void (*cb)( void *aux ), void *aux )
+{
+	ctx->bad_callback = cb;
+	ctx->bad_callback_aux = aux;
+}
+
 typedef struct {
 	char *data;
 	int len;
@@ -168,8 +184,7 @@ typedef struct {
 #define DRV_OK          0
 #define DRV_MSG_BAD     1
 #define DRV_BOX_BAD     2
-#define DRV_STORE_BAD   3
-#define DRV_CANCELED    4
+#define DRV_CANCELED    3
 
 /* All memory belongs to the driver's user. */
 

src/main.c

@@ -672,6 +672,17 @@ sync_chans( main_vars_t *mvars, int ent )
 		drivers[t]->cleanup();
 }
 
+static void
+store_bad( void *aux )
+{
+	MVARS(aux)
+
+	mvars->drv[t]->cancel_store( mvars->ctx[t] );
+	mvars->ret = mvars->skip = 1;
+	mvars->state[t] = ST_CLOSED;
+	sync_chans( mvars, E_OPEN );
+}
+
 static void
 store_opened( store_t *ctx, void *aux )
 {
@@ -685,6 +696,7 @@ store_opened( store_t *ctx, void *aux )
 	}
 	mvars->ctx[t] = ctx;
 	if (!mvars->skip && !mvars->boxlist && mvars->chan->patterns && !ctx->listed) {
+		set_bad_callback( ctx, store_bad, AUX );
 		mvars->drv[t]->list( ctx, store_listed, AUX );
 	} else {
 		mvars->state[t] = ST_OPEN;
@@ -697,20 +709,19 @@ store_listed( int sts, void *aux )
 {
 	MVARS(aux)
 
-	mvars->state[t] = ST_OPEN;
 	switch (sts) {
+	case DRV_CANCELED:
+		return;
 	case DRV_OK:
 		mvars->ctx[t]->listed = 1;
 		if (mvars->ctx[t]->conf->map_inbox)
 			add_string_list( &mvars->ctx[t]->boxes, mvars->ctx[t]->conf->map_inbox );
 		break;
-	case DRV_STORE_BAD:
-		mvars->drv[t]->cancel_store( mvars->ctx[t] );
-		mvars->state[t] = ST_CLOSED;
 	default:
 		mvars->ret = mvars->skip = 1;
 		break;
 	}
+	mvars->state[t] = ST_OPEN;
 	sync_chans( mvars, E_OPEN );
 }
 

src/sync.c

@@ -331,10 +331,6 @@ msg_fetched( int sts, void *aux )
 		return vars->cb( SYNC_CANCELED, 0, vars );
 	case DRV_MSG_BAD:
 		return vars->cb( SYNC_NOGOOD, 0, vars );
-	case DRV_STORE_BAD:
-		INIT_SVARS(vars->aux);
-		(void)svars;
-		return vars->cb( SYNC_BAD(1-t), 0, vars );
 	default:
 		return vars->cb( SYNC_FAIL, 0, vars );
 	}
@@ -357,10 +353,6 @@ msg_stored( int sts, int uid, void *aux )
 		warn( "Warning: %s refuses to store message %d from %s.\n",
 		      str_ms[t], vars->msg->uid, str_ms[1-t] );
 		return vars->cb( SYNC_NOGOOD, 0, vars );
-	case DRV_STORE_BAD:
-		INIT_SVARS(vars->aux);
-		(void)svars;
-		return vars->cb( SYNC_BAD(t), 0, vars );
 	default:
 		return vars->cb( SYNC_FAIL, 0, vars );
 	}
@@ -406,7 +398,6 @@ cancel_sync( sync_vars_t *svars )
 	for (t = 0; t < 2; t++) {
 		int other_state = svars->state[1-t];
 		if (svars->ret & SYNC_BAD(t)) {
-			svars->drv[t]->cancel_store( svars->ctx[t] );
 			cancel_done( AUX );
 		} else {
 			svars->drv[t]->cancel( svars->ctx[t], cancel_done, AUX );
@@ -429,6 +420,16 @@ cancel_done( void *aux )
 	}
 }
 
+static void
+store_bad( void *aux )
+{
+	DECL_INIT_SVARS(aux);
+
+	svars->drv[t]->cancel_store( svars->ctx[t] );
+	svars->ret |= SYNC_BAD(t);
+	cancel_sync( svars );
+}
+
 
 static int
 check_ret( int sts, void *aux )
@@ -438,11 +439,6 @@ check_ret( int sts, void *aux )
 	switch (sts) {
 	case DRV_CANCELED:
 		return 1;
-	case DRV_STORE_BAD:
-		INIT_SVARS(aux);
-		svars->ret |= SYNC_BAD(t);
-		cancel_sync( svars );
-		return 1;
 	case DRV_BOX_BAD:
 		INIT_SVARS(aux);
 		svars->ret |= SYNC_FAIL;
@@ -522,6 +518,7 @@ sync_boxes( store_t *ctx[], const char *names[], channel_conf_t *chan,
 			(!names[t] || (ctx[t]->conf->map_inbox && !strcmp( ctx[t]->conf->map_inbox, names[t] ))) ?
 				"INBOX" : names[t];
 		ctx[t]->uidvalidity = -1;
+		set_bad_callback( ctx[t], store_bad, AUX );
 		svars->drv[t] = ctx[t]->conf->driver;
 		svars->drv[t]->prepare_paths( ctx[t] );
 	}