From b0bbd23512ff41608adc07dec77abf97b58012ba Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 15 Jul 2012 12:55:04 +0200 Subject: [PATCH] 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 | 98 +++++++++++++++++++++++++++++++---------------- src/drv_maildir.c | 22 +++++++---- src/isync.h | 19 ++++++++- src/main.c | 19 +++++++-- src/sync.c | 25 ++++++------ 5 files changed, 122 insertions(+), 61 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index e60a3f1..9ad1f03 100644 --- a/src/drv_imap.c +++ b/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 ", 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 diff --git a/src/drv_maildir.c b/src/drv_maildir.c index 48474fc..91822d9 100644 --- a/src/drv_maildir.c +++ b/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; diff --git a/src/isync.h b/src/isync.h index c831b77..c1c6179 100644 --- a/src/isync.h +++ b/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. */ diff --git a/src/main.c b/src/main.c index 1c5f2e1..9683462 100644 --- a/src/main.c +++ b/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 ); } diff --git a/src/sync.c b/src/sync.c index 865cd67..dd0a662 100644 --- a/src/sync.c +++ b/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] ); }