fix error paths wrt sync drivers, take 3
msgs_copied() was not checked at all, and msgs_flags_set() was doing it wrong (sync_close() was not checked). instead of trying to fix/extend the msgs_flags_set() model (ref-counting and cancelation checking in lower-level functions, and return values to propagate the status), place the refs/derefs around higher-level scopes and do the checking only there. this is effectively simpler, and does away with some obscure macros.
This commit is contained in:
parent
2f0fbcd306
commit
c47ee1c8c4
98
src/sync.c
98
src/sync.c
|
@ -166,25 +166,9 @@ typedef struct {
|
||||||
} sync_vars_t;
|
} sync_vars_t;
|
||||||
|
|
||||||
static void sync_ref( sync_vars_t *svars ) { ++svars->ref_count; }
|
static void sync_ref( sync_vars_t *svars ) { ++svars->ref_count; }
|
||||||
static int sync_deref( sync_vars_t *svars );
|
static void sync_deref( sync_vars_t *svars );
|
||||||
static int deref_check_cancel( sync_vars_t *svars );
|
|
||||||
static int check_cancel( sync_vars_t *svars );
|
static int check_cancel( sync_vars_t *svars );
|
||||||
|
|
||||||
#define DRIVER_CALL_RET(call) \
|
|
||||||
do { \
|
|
||||||
sync_ref( svars ); \
|
|
||||||
svars->drv[t]->call; \
|
|
||||||
return deref_check_cancel( svars ); \
|
|
||||||
} while (0)
|
|
||||||
|
|
||||||
#define DRIVER_CALL(call) \
|
|
||||||
do { \
|
|
||||||
sync_ref( svars ); \
|
|
||||||
svars->drv[t]->call; \
|
|
||||||
if (deref_check_cancel( svars )) \
|
|
||||||
return; \
|
|
||||||
} while (0)
|
|
||||||
|
|
||||||
#define AUX &svars->t[t]
|
#define AUX &svars->t[t]
|
||||||
#define INV_AUX &svars->t[1-t]
|
#define INV_AUX &svars->t[1-t]
|
||||||
#define DECL_SVARS \
|
#define DECL_SVARS \
|
||||||
|
@ -282,7 +266,7 @@ typedef struct copy_vars {
|
||||||
|
|
||||||
static void msg_fetched( int sts, void *aux );
|
static void msg_fetched( int sts, void *aux );
|
||||||
|
|
||||||
static int
|
static void
|
||||||
copy_msg( copy_vars_t *vars )
|
copy_msg( copy_vars_t *vars )
|
||||||
{
|
{
|
||||||
DECL_INIT_SVARS(vars->aux);
|
DECL_INIT_SVARS(vars->aux);
|
||||||
|
@ -290,7 +274,7 @@ copy_msg( copy_vars_t *vars )
|
||||||
t ^= 1;
|
t ^= 1;
|
||||||
vars->data.flags = vars->msg->flags;
|
vars->data.flags = vars->msg->flags;
|
||||||
vars->data.date = svars->chan->use_internal_date ? -1 : 0;
|
vars->data.date = svars->chan->use_internal_date ? -1 : 0;
|
||||||
DRIVER_CALL_RET(fetch_msg( svars->ctx[t], vars->msg, &vars->data, msg_fetched, vars ));
|
svars->drv[t]->fetch_msg( svars->ctx[t], vars->msg, &vars->data, msg_fetched, vars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static void msg_stored( int sts, int uid, void *aux );
|
static void msg_stored( int sts, int uid, void *aux );
|
||||||
|
@ -533,14 +517,6 @@ store_bad( void *aux )
|
||||||
cancel_sync( svars );
|
cancel_sync( svars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static int
|
|
||||||
deref_check_cancel( sync_vars_t *svars )
|
|
||||||
{
|
|
||||||
if (sync_deref( svars ))
|
|
||||||
return -1;
|
|
||||||
return check_cancel( svars );
|
|
||||||
}
|
|
||||||
|
|
||||||
static int
|
static int
|
||||||
check_cancel( sync_vars_t *svars )
|
check_cancel( sync_vars_t *svars )
|
||||||
{
|
{
|
||||||
|
@ -640,13 +616,17 @@ sync_boxes( store_t *ctx[], const char *names[], channel_conf_t *chan,
|
||||||
}
|
}
|
||||||
/* Both boxes must be fully set up at this point, so that error exit paths
|
/* Both boxes must be fully set up at this point, so that error exit paths
|
||||||
* don't run into uninitialized variables. */
|
* don't run into uninitialized variables. */
|
||||||
|
sync_ref( svars );
|
||||||
for (t = 0; t < 2; t++) {
|
for (t = 0; t < 2; t++) {
|
||||||
info( "Selecting %s %s...\n", str_ms[t], ctx[t]->orig_name );
|
info( "Selecting %s %s...\n", str_ms[t], ctx[t]->orig_name );
|
||||||
DRIVER_CALL(select( ctx[t], (chan->ops[t] & OP_CREATE) != 0, box_selected, AUX ));
|
svars->drv[t]->select( ctx[t], (chan->ops[t] & OP_CREATE) != 0, box_selected, AUX );
|
||||||
|
if (check_cancel( svars ))
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
|
sync_deref( svars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static int load_box( sync_vars_t *svars, int t, int minwuid, int *mexcs, int nmexcs );
|
static void load_box( sync_vars_t *svars, int t, int minwuid, int *mexcs, int nmexcs );
|
||||||
|
|
||||||
static void
|
static void
|
||||||
box_selected( int sts, void *aux )
|
box_selected( int sts, void *aux )
|
||||||
|
@ -1083,14 +1063,16 @@ box_selected( int sts, void *aux )
|
||||||
} else {
|
} else {
|
||||||
minwuid = INT_MAX;
|
minwuid = INT_MAX;
|
||||||
}
|
}
|
||||||
if (load_box( svars, M, minwuid, mexcs, nmexcs ))
|
sync_ref( svars );
|
||||||
return;
|
load_box( svars, M, minwuid, mexcs, nmexcs );
|
||||||
load_box( svars, S, (ctx[S]->opts & OPEN_OLD) ? 1 : INT_MAX, 0, 0 );
|
if (!check_cancel( svars ))
|
||||||
|
load_box( svars, S, (ctx[S]->opts & OPEN_OLD) ? 1 : INT_MAX, 0, 0 );
|
||||||
|
sync_deref( svars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static void box_loaded( int sts, void *aux );
|
static void box_loaded( int sts, void *aux );
|
||||||
|
|
||||||
static int
|
static void
|
||||||
load_box( sync_vars_t *svars, int t, int minwuid, int *mexcs, int nmexcs )
|
load_box( sync_vars_t *svars, int t, int minwuid, int *mexcs, int nmexcs )
|
||||||
{
|
{
|
||||||
sync_rec_t *srec;
|
sync_rec_t *srec;
|
||||||
|
@ -1109,7 +1091,7 @@ load_box( sync_vars_t *svars, int t, int minwuid, int *mexcs, int nmexcs )
|
||||||
maxwuid = 0;
|
maxwuid = 0;
|
||||||
info( "Loading %s...\n", str_ms[t] );
|
info( "Loading %s...\n", str_ms[t] );
|
||||||
debug( maxwuid == INT_MAX ? "loading %s [%d,inf]\n" : "loading %s [%d,%d]\n", str_ms[t], minwuid, maxwuid );
|
debug( maxwuid == INT_MAX ? "loading %s [%d,inf]\n" : "loading %s [%d,%d]\n", str_ms[t], minwuid, maxwuid );
|
||||||
DRIVER_CALL_RET(load( svars->ctx[t], minwuid, maxwuid, svars->newuid[t], mexcs, nmexcs, box_loaded, AUX ));
|
svars->drv[t]->load( svars->ctx[t], minwuid, maxwuid, svars->newuid[t], mexcs, nmexcs, box_loaded, AUX );
|
||||||
}
|
}
|
||||||
|
|
||||||
typedef struct {
|
typedef struct {
|
||||||
|
@ -1125,7 +1107,7 @@ typedef struct {
|
||||||
|
|
||||||
static void flags_set( int sts, void *aux );
|
static void flags_set( int sts, void *aux );
|
||||||
static void flags_set_p2( sync_vars_t *svars, sync_rec_t *srec, int t );
|
static void flags_set_p2( sync_vars_t *svars, sync_rec_t *srec, int t );
|
||||||
static int msgs_flags_set( sync_vars_t *svars, int t );
|
static void msgs_flags_set( sync_vars_t *svars, int t );
|
||||||
static void msg_copied( int sts, int uid, copy_vars_t *vars );
|
static void msg_copied( int sts, int uid, copy_vars_t *vars );
|
||||||
static void msg_copied_p2( sync_vars_t *svars, sync_rec_t *srec, int t, int uid );
|
static void msg_copied_p2( sync_vars_t *svars, sync_rec_t *srec, int t, int uid );
|
||||||
static void msgs_copied( sync_vars_t *svars, int t );
|
static void msgs_copied( sync_vars_t *svars, int t );
|
||||||
|
@ -1459,6 +1441,8 @@ box_loaded( int sts, void *aux )
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
sync_ref( svars );
|
||||||
|
|
||||||
debug( "synchronizing flags\n" );
|
debug( "synchronizing flags\n" );
|
||||||
for (srec = svars->srecs; srec; srec = srec->next) {
|
for (srec = svars->srecs; srec; srec = srec->next) {
|
||||||
if ((srec->status & S_DEAD) || srec->uid[M] <= 0 || srec->uid[S] <= 0)
|
if ((srec->status & S_DEAD) || srec->uid[M] <= 0 || srec->uid[S] <= 0)
|
||||||
|
@ -1502,7 +1486,9 @@ box_loaded( int sts, void *aux )
|
||||||
fv->srec = srec;
|
fv->srec = srec;
|
||||||
fv->aflags = aflags;
|
fv->aflags = aflags;
|
||||||
fv->dflags = dflags;
|
fv->dflags = dflags;
|
||||||
DRIVER_CALL(set_flags( svars->ctx[t], srec->msg[t], srec->uid[t], aflags, dflags, flags_set, fv ));
|
svars->drv[t]->set_flags( svars->ctx[t], srec->msg[t], srec->uid[t], aflags, dflags, flags_set, fv );
|
||||||
|
if (check_cancel( svars ))
|
||||||
|
goto out;
|
||||||
} else
|
} else
|
||||||
flags_set_p2( svars, srec, t );
|
flags_set_p2( svars, srec, t );
|
||||||
}
|
}
|
||||||
|
@ -1510,8 +1496,9 @@ box_loaded( int sts, void *aux )
|
||||||
for (t = 0; t < 2; t++) {
|
for (t = 0; t < 2; t++) {
|
||||||
svars->drv[t]->commit( svars->ctx[t] );
|
svars->drv[t]->commit( svars->ctx[t] );
|
||||||
svars->state[t] |= ST_SENT_FLAGS;
|
svars->state[t] |= ST_SENT_FLAGS;
|
||||||
if (msgs_flags_set( svars, t ))
|
msgs_flags_set( svars, t );
|
||||||
return;
|
if (check_cancel( svars ))
|
||||||
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
debug( "propagating new messages\n" );
|
debug( "propagating new messages\n" );
|
||||||
|
@ -1528,13 +1515,19 @@ box_loaded( int sts, void *aux )
|
||||||
cv->aux = AUX;
|
cv->aux = AUX;
|
||||||
cv->srec = srec;
|
cv->srec = srec;
|
||||||
cv->msg = tmsg;
|
cv->msg = tmsg;
|
||||||
if (copy_msg( cv ))
|
copy_msg( cv );
|
||||||
return;
|
if (check_cancel( svars ))
|
||||||
|
goto out;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
svars->state[t] |= ST_SENT_NEW;
|
svars->state[t] |= ST_SENT_NEW;
|
||||||
msgs_copied( svars, t );
|
msgs_copied( svars, t );
|
||||||
|
if (check_cancel( svars ))
|
||||||
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
out:
|
||||||
|
sync_deref( svars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
|
@ -1687,14 +1680,16 @@ flags_set_p2( sync_vars_t *svars, sync_rec_t *srec, int t )
|
||||||
static void msg_trashed( int sts, void *aux );
|
static void msg_trashed( int sts, void *aux );
|
||||||
static void msg_rtrashed( int sts, int uid, copy_vars_t *vars );
|
static void msg_rtrashed( int sts, int uid, copy_vars_t *vars );
|
||||||
|
|
||||||
static int
|
static void
|
||||||
msgs_flags_set( sync_vars_t *svars, int t )
|
msgs_flags_set( sync_vars_t *svars, int t )
|
||||||
{
|
{
|
||||||
message_t *tmsg;
|
message_t *tmsg;
|
||||||
copy_vars_t *cv;
|
copy_vars_t *cv;
|
||||||
|
|
||||||
if (!(svars->state[t] & ST_SENT_FLAGS) || svars->flags_done[t] < svars->flags_total[t])
|
if (!(svars->state[t] & ST_SENT_FLAGS) || svars->flags_done[t] < svars->flags_total[t])
|
||||||
return 0;
|
return;
|
||||||
|
|
||||||
|
sync_ref( svars );
|
||||||
|
|
||||||
if ((svars->chan->ops[t] & OP_EXPUNGE) &&
|
if ((svars->chan->ops[t] & OP_EXPUNGE) &&
|
||||||
(svars->ctx[t]->conf->trash || (svars->ctx[1-t]->conf->trash && svars->ctx[1-t]->conf->trash_remote_new))) {
|
(svars->ctx[t]->conf->trash || (svars->ctx[1-t]->conf->trash && svars->ctx[1-t]->conf->trash_remote_new))) {
|
||||||
|
@ -1706,10 +1701,9 @@ msgs_flags_set( sync_vars_t *svars, int t )
|
||||||
debug( "%s: trashing message %d\n", str_ms[t], tmsg->uid );
|
debug( "%s: trashing message %d\n", str_ms[t], tmsg->uid );
|
||||||
svars->trash_total[t]++;
|
svars->trash_total[t]++;
|
||||||
stats( svars );
|
stats( svars );
|
||||||
sync_ref( svars );
|
|
||||||
svars->drv[t]->trash_msg( svars->ctx[t], tmsg, msg_trashed, AUX );
|
svars->drv[t]->trash_msg( svars->ctx[t], tmsg, msg_trashed, AUX );
|
||||||
if (deref_check_cancel( svars ))
|
if (check_cancel( svars ))
|
||||||
return -1;
|
goto out;
|
||||||
} else
|
} else
|
||||||
debug( "%s: not trashing message %d - not new\n", str_ms[t], tmsg->uid );
|
debug( "%s: not trashing message %d - not new\n", str_ms[t], tmsg->uid );
|
||||||
} else {
|
} else {
|
||||||
|
@ -1723,8 +1717,9 @@ msgs_flags_set( sync_vars_t *svars, int t )
|
||||||
cv->aux = INV_AUX;
|
cv->aux = INV_AUX;
|
||||||
cv->srec = 0;
|
cv->srec = 0;
|
||||||
cv->msg = tmsg;
|
cv->msg = tmsg;
|
||||||
if (copy_msg( cv ))
|
copy_msg( cv );
|
||||||
return -1;
|
if (check_cancel( svars ))
|
||||||
|
goto out;
|
||||||
} else
|
} else
|
||||||
debug( "%s: not remote trashing message %d - too big\n", str_ms[t], tmsg->uid );
|
debug( "%s: not remote trashing message %d - too big\n", str_ms[t], tmsg->uid );
|
||||||
} else
|
} else
|
||||||
|
@ -1734,7 +1729,9 @@ msgs_flags_set( sync_vars_t *svars, int t )
|
||||||
}
|
}
|
||||||
svars->state[t] |= ST_SENT_TRASH;
|
svars->state[t] |= ST_SENT_TRASH;
|
||||||
sync_close( svars, t );
|
sync_close( svars, t );
|
||||||
return 0;
|
|
||||||
|
out:
|
||||||
|
sync_deref( svars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
|
@ -1914,7 +1911,8 @@ sync_bail3( sync_vars_t *svars )
|
||||||
sync_deref( svars );
|
sync_deref( svars );
|
||||||
}
|
}
|
||||||
|
|
||||||
static int sync_deref( sync_vars_t *svars )
|
static void
|
||||||
|
sync_deref( sync_vars_t *svars )
|
||||||
{
|
{
|
||||||
if (!--svars->ref_count) {
|
if (!--svars->ref_count) {
|
||||||
void (*cb)( int sts, void *aux ) = svars->cb;
|
void (*cb)( int sts, void *aux ) = svars->cb;
|
||||||
|
@ -1922,7 +1920,5 @@ static int sync_deref( sync_vars_t *svars )
|
||||||
int ret = svars->ret;
|
int ret = svars->ret;
|
||||||
free( svars );
|
free( svars );
|
||||||
cb( ret, aux );
|
cb( ret, aux );
|
||||||
return -1;
|
|
||||||
}
|
}
|
||||||
return 0;
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue
Block a user