commit c47ee1c8c4bc1961cf7fd665326bfa05504ab0e7
Author: Oswald Buddenhagen <[email protected]>
Date:   Wed Dec 11 16:13:49 2013 +0100

    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.

 src/sync.c |   98 +++++++++++++++++++++++++---------------------------
 1 files changed, 47 insertions(+), 51 deletions(-)

diff --git a/src/sync.c b/src/sync.c
index bc55911..94b3fb7 100644
--- a/src/sync.c
+++ b/src/sync.c
@@ -166,25 +166,9 @@ typedef struct {
 } sync_vars_t;
 
 static void sync_ref( sync_vars_t *svars ) { ++svars->ref_count; }
-static int sync_deref( sync_vars_t *svars );
-static int deref_check_cancel( sync_vars_t *svars );
+static void sync_deref( 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 INV_AUX &svars->t[1-t]
 #define DECL_SVARS \
@@ -282,7 +266,7 @@ typedef struct copy_vars {
 
 static void msg_fetched( int sts, void *aux );
 
-static int
+static void
 copy_msg( copy_vars_t *vars )
 {
        DECL_INIT_SVARS(vars->aux);
@@ -290,7 +274,7 @@ copy_msg( copy_vars_t *vars )
        t ^= 1;
        vars->data.flags = vars->msg->flags;
        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 );
@@ -534,14 +518,6 @@ store_bad( void *aux )
 }
 
 static int
-deref_check_cancel( sync_vars_t *svars )
-{
-       if (sync_deref( svars ))
-               return -1;
-       return check_cancel( svars );
-}
-
-static int
 check_cancel( sync_vars_t *svars )
 {
        return (svars->state[M] | svars->state[S]) & (ST_SENT_CANCEL | 
ST_CANCELED);
@@ -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
         * don't run into uninitialized variables. */
+       sync_ref( svars );
        for (t = 0; t < 2; t++) {
                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
 box_selected( int sts, void *aux )
@@ -1083,14 +1063,16 @@ box_selected( int sts, void *aux )
        } else {
                minwuid = INT_MAX;
        }
-       if (load_box( svars, M, minwuid, mexcs, nmexcs ))
-               return;
-       load_box( svars, S, (ctx[S]->opts & OPEN_OLD) ? 1 : INT_MAX, 0, 0 );
+       sync_ref( svars );
+       load_box( svars, M, minwuid, mexcs, nmexcs );
+       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 int
+static void
 load_box( sync_vars_t *svars, int t, int minwuid, int *mexcs, int nmexcs )
 {
        sync_rec_t *srec;
@@ -1109,7 +1091,7 @@ load_box( sync_vars_t *svars, int t, int minwuid, int 
*mexcs, int nmexcs )
                maxwuid = 0;
        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 );
-       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 {
@@ -1125,7 +1107,7 @@ typedef struct {
 
 static void flags_set( int sts, void *aux );
 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_p2( sync_vars_t *svars, sync_rec_t *srec, int t, int 
uid );
 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" );
        for (srec = svars->srecs; srec; srec = srec->next) {
                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->aflags = aflags;
                                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
                                flags_set_p2( svars, srec, t );
                }
@@ -1510,8 +1496,9 @@ box_loaded( int sts, void *aux )
        for (t = 0; t < 2; t++) {
                svars->drv[t]->commit( svars->ctx[t] );
                svars->state[t] |= ST_SENT_FLAGS;
-               if (msgs_flags_set( svars, t ))
-                       return;
+               msgs_flags_set( svars, t );
+               if (check_cancel( svars ))
+                       goto out;
        }
 
        debug( "propagating new messages\n" );
@@ -1528,13 +1515,19 @@ box_loaded( int sts, void *aux )
                                cv->aux = AUX;
                                cv->srec = srec;
                                cv->msg = tmsg;
-                               if (copy_msg( cv ))
-                                       return;
+                               copy_msg( cv );
+                               if (check_cancel( svars ))
+                                       goto out;
                        }
                }
                svars->state[t] |= ST_SENT_NEW;
                msgs_copied( svars, t );
+               if (check_cancel( svars ))
+                       goto out;
        }
+
+  out:
+       sync_deref( svars );
 }
 
 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_rtrashed( int sts, int uid, copy_vars_t *vars );
 
-static int
+static void
 msgs_flags_set( sync_vars_t *svars, int t )
 {
        message_t *tmsg;
        copy_vars_t *cv;
 
        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) &&
            (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 );
                                                svars->trash_total[t]++;
                                                stats( svars );
-                                               sync_ref( svars );
                                                svars->drv[t]->trash_msg( 
svars->ctx[t], tmsg, msg_trashed, AUX );
-                                               if (deref_check_cancel( svars ))
-                                                       return -1;
+                                               if (check_cancel( svars ))
+                                                       goto out;
                                        } else
                                                debug( "%s: not trashing 
message %d - not new\n", str_ms[t], tmsg->uid );
                                } else {
@@ -1723,8 +1717,9 @@ msgs_flags_set( sync_vars_t *svars, int t )
                                                        cv->aux = INV_AUX;
                                                        cv->srec = 0;
                                                        cv->msg = tmsg;
-                                                       if (copy_msg( cv ))
-                                                               return -1;
+                                                       copy_msg( cv );
+                                                       if (check_cancel( svars 
))
+                                                               goto out;
                                                } else
                                                        debug( "%s: not remote 
trashing message %d - too big\n", str_ms[t], tmsg->uid );
                                        } else
@@ -1734,7 +1729,9 @@ msgs_flags_set( sync_vars_t *svars, int t )
        }
        svars->state[t] |= ST_SENT_TRASH;
        sync_close( svars, t );
-       return 0;
+
+  out:
+       sync_deref( svars );
 }
 
 static void
@@ -1914,7 +1911,8 @@ sync_bail3( sync_vars_t *svars )
        sync_deref( svars );
 }
 
-static int sync_deref( sync_vars_t *svars )
+static void
+sync_deref( sync_vars_t *svars )
 {
        if (!--svars->ref_count) {
                void (*cb)( int sts, void *aux ) = svars->cb;
@@ -1922,7 +1920,5 @@ static int sync_deref( sync_vars_t *svars )
                int ret = svars->ret;
                free( svars );
                cb( ret, aux );
-               return -1;
        }
-       return 0;
 }

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
isync-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/isync-devel

Reply via email to