Changeset 691:78b665400097 in freeDiameter for libfdcore

Timestamp:

Jan 20, 2011, 7:44:27 PM (13 years ago)

Author:

Sebastien Decugis <sdecugis@nict.go.jp>

Branch:

default

Phase:

public

Message:

Cleanup all pthread_cleanup_push / pop pairs so that pop is always called after push, or ASSERT(0) is some grave errors

Location:

libfdcore

Files:

• core.c (modified) (3 diffs)
• p_cnx.c (modified) (4 diffs)
• p_expiry.c (modified) (5 diffs)
• p_out.c (modified) (1 diff)
• p_psm.c (modified) (1 diff)
• p_sr.c (modified) (2 diffs)
• peers.c (modified) (1 diff)
• routing_dispatch.c (modified) (1 diff)
• sctp.c (modified) (3 diffs)
• server.c (modified) (1 diff)
• tcp.c (modified) (1 diff)

libfdcore/core.c

@@ -241 +241 @@
 int fd_core_waitstartcomplete(void)
 {
+        int ret = 0;
+        
         TRACE_ENTRY("");
         
@@ -246 +248 @@
         pthread_cleanup_push( fd_cleanup_mutex, &is_ready_mtx );
         
-        while (!is_ready) {
-                CHECK_POSIX( pthread_cond_wait( &is_ready_cnd, &is_ready_mtx ) );
+        while (!ret && !is_ready) {
+                CHECK_POSIX_DO( ret = pthread_cond_wait( &is_ready_cnd, &is_ready_mtx ),  );
         }
         
@@ -253 +255 @@
         CHECK_POSIX( pthread_mutex_unlock( &is_ready_mtx ) );
         
-        return 0;
+        return ret;
 }
 

libfdcore/p_cnx.c

@@ -211 +211 @@
         struct next_conn * nc = NULL;
         int rebuilt = 0;
+        int fatal_error=0;
         
         TRACE_ENTRY("%p", arg);
@@ -226 +227 @@
                 if (FD_IS_LIST_EMPTY(&peer->p_connparams)) {
                         if (! rebuilt) {
-                                CHECK_FCT_DO( prepare_connection_list(peer), goto fatal_error );
+                                CHECK_FCT_DO( fatal_error = prepare_connection_list(peer), goto out );
                                 rebuilt ++;
                         }
@@ -232 +233 @@
                                 /* We encountered an error or we have looped over all the addresses of the peer. */
                                 TRACE_DEBUG(INFO, "Unable to connect to the peer %s, aborting attempts for now.", peer->p_hdr.info.pi_diamid);
-                                CHECK_FCT_DO( fd_event_send(peer->p_events, FDEVP_CNX_FAILED, 0, NULL), goto fatal_error );
+                                CHECK_FCT_DO( fatal_error = fd_event_send(peer->p_events, FDEVP_CNX_FAILED, 0, NULL), goto out );
                                 return NULL;
                         }
@@ -277 +278 @@
                                 empty_connection_list(peer);
                                 fd_ep_filter(&peer->p_hdr.info.pi_endpoints, EP_FL_CONF);
-                                return NULL;
+                                goto out_pop;
                         } );
         } else {
                 /* Prepare to receive the next message */
-                CHECK_FCT_DO( fd_cnx_start_clear(cnx, 0), goto fatal_error );
+                CHECK_FCT_DO( fatal_error = fd_cnx_start_clear(cnx, 0), goto out_pop );
         }
         
         /* Upon success, generate FDEVP_CNX_ESTABLISHED */
-        CHECK_FCT_DO( fd_event_send(peer->p_events, FDEVP_CNX_ESTABLISHED, 0, cnx), goto fatal_error );
-        
+        CHECK_FCT_DO( fatal_error = fd_event_send(peer->p_events, FDEVP_CNX_ESTABLISHED, 0, cnx),  );
+out_pop:
+        ;       
         pthread_cleanup_pop(0);
         
-        return NULL;
-        
-fatal_error:
-        /* Cleanup the connection */
-        if (cnx)
-                fd_cnx_destroy(cnx);
-
-        /* Generate a termination event */
-        CHECK_FCT_DO(fd_event_send(fd_g_config->cnf_main_ev, FDEV_TERMINATE, 0, NULL), );
+out:
+        
+        if (fatal_error) {
+        
+                /* Cleanup the connection */
+                if (cnx)
+                        fd_cnx_destroy(cnx);
+
+                /* Generate a termination event */
+                CHECK_FCT_DO(fd_event_send(fd_g_config->cnf_main_ev, FDEV_TERMINATE, 0, NULL), );
+        }
         
         return NULL;

libfdcore/p_expiry.c

@@ -98 +98 @@
         TRACE_ENTRY( "%p", arg );
         
-        CHECK_POSIX_DO( pthread_mutex_lock(&exp_mtx),  goto error );
+        CHECK_POSIX_DO( pthread_mutex_lock(&exp_mtx), { ASSERT(0); } );
         pthread_cleanup_push( fd_cleanup_mutex, &exp_mtx );
         
@@ -108 +108 @@
                 if (FD_IS_LIST_EMPTY(&exp_list)) {
                         /* Just wait for a change or cancelation */
-                        CHECK_POSIX_DO( pthread_cond_wait( &exp_cnd, &exp_mtx ), goto error );
+                        CHECK_POSIX_DO( pthread_cond_wait( &exp_cnd, &exp_mtx ), { ASSERT(0); } );
                         /* Restart the loop on wakeup */
                         continue;
@@ -118 +118 @@
                 
                 /* Get the current time */
-                CHECK_SYS_DO(  clock_gettime(CLOCK_REALTIME, &now),  goto error  );
+                CHECK_SYS_DO(  clock_gettime(CLOCK_REALTIME, &now),  { ASSERT(0); }  );
 
                 /* If first peer is not expired, we just wait until it happens */
@@ -125 +125 @@
                         CHECK_POSIX_DO2(  pthread_cond_timedwait( &exp_cnd, &exp_mtx, &first->p_exp_timer ),  
                                         ETIMEDOUT, /* ETIMEDOUT is a normal return value, continue */,
-                                        /* on other error, */ goto error );
+                                        /* on other error, */ { ASSERT(0); } );
         
                         /* on wakeup, loop */
@@ -133 +133 @@
                 /* Now, the first peer in the list is expired; signal it */
                 fd_list_unlink( &first->p_expiry );
-                CHECK_FCT_DO( fd_event_send(first->p_events, FDEVP_TERMINATE, 0, "DO_NOT_WANT_TO_TALK_TO_YOU"), goto error );
+                CHECK_FCT_DO( fd_event_send(first->p_events, FDEVP_TERMINATE, 0, "DO_NOT_WANT_TO_TALK_TO_YOU"), break );
                 
         } while (1);
         
         pthread_cleanup_pop( 1 );
-error:
+
         TRACE_DEBUG(INFO, "An error occurred in peers module! Expiry thread is terminating...");
-        ASSERT(0);
         CHECK_FCT_DO(fd_event_send(fd_g_config->cnf_main_ev, FDEV_TERMINATE, 0, NULL), );
         return NULL;

libfdcore/p_out.c

@@ -69 +69 @@
         /* Save a request before sending so that there is no race condition with the answer */
         if (msg_is_a_req) {
-                CHECK_FCT_DO( ret = fd_p_sr_store(srl, msg, &hdr->msg_hbhid, bkp_hbh), { free(buf); return ret; } );
+                CHECK_FCT_DO( ret = fd_p_sr_store(srl, msg, &hdr->msg_hbhid, bkp_hbh), goto out );
         }
         
         /* Send the message */
-        CHECK_FCT_DO( ret = fd_cnx_send(cnx, buf, sz, flags), { free(buf); return ret; } );
+        CHECK_FCT_DO( ret = fd_cnx_send(cnx, buf, sz, flags), );
+out:
+        ;       
         pthread_cleanup_pop(1);
+        
+        if (ret)
+                return ret;
         
         /* Free remaining messages (i.e. answers) */

libfdcore/p_psm.c

@@ -55 +55 @@
 static int fd_psm_waitstart()
 {
+        int ret = 0;
         TRACE_ENTRY("");
         CHECK_POSIX( pthread_mutex_lock(&started_mtx) );
 awake:  
-        if (! started) {
+        if (!ret && !started) {
                 pthread_cleanup_push( fd_cleanup_mutex, &started_mtx );
-                CHECK_POSIX( pthread_cond_wait(&started_cnd, &started_mtx) );
+                CHECK_POSIX_DO( ret = pthread_cond_wait(&started_cnd, &started_mtx), );
                 pthread_cleanup_pop( 0 );
                 goto awake;
         }
         CHECK_POSIX( pthread_mutex_unlock(&started_mtx) );
-        return 0;
+        return ret;
 }
 

libfdcore/p_sr.c

@@ -117 +117 @@
 }
 
-/* thread that handles messages expiring. The thread is started / stopped only when needed */
+/* thread that handles messages expiring. The thread is started / cancelled only when needed */
 static void * sr_expiry_th(void * arg) {
         struct sr_list * srlist = arg;
@@ -184 +184 @@
         ; /* pthread_cleanup_pop sometimes expands as "} ..." and the label beofre this cause some compilers to complain... */
         pthread_cleanup_pop( 1 );
+        ASSERT(0); /* we have encountered a problem, maybe time to signal the framework to terminate? */
         return NULL;
 }

libfdcore/peers.c

@@ -533 +533 @@
                 int auth = 0;
                 pthread_cleanup_push(fd_cleanup_rwlock, &validators_rw);
-                CHECK_FCT_DO( ret = ((int(*)(struct peer_info *, int *, int (**)(struct peer_info *)))(v->o)) (&peer->p_hdr.info, &auth, &peer->p_cb2), goto out );
+                CHECK_FCT_DO( ret = ((int(*)(struct peer_info *, int *, int (**)(struct peer_info *)))(v->o)) (&peer->p_hdr.info, &auth, &peer->p_cb2),  );
                 pthread_cleanup_pop(0);
+                if (ret)
+                        goto out;
                 if (auth) {
                         ret = (auth > 0) ? 0 : -1;

libfdcore/routing_dispatch.c

@@ -1000 +1000 @@
                 {
                         int must_stop;
-                        CHECK_POSIX_DO( pthread_mutex_lock(&order_lock), goto end ); /* we lock to flush the caches */
+                        CHECK_POSIX_DO( pthread_mutex_lock(&order_lock), { ASSERT(0); } ); /* we lock to flush the caches */
                         must_stop = (order_val == STOP);
-                        CHECK_POSIX_DO( pthread_mutex_unlock(&order_lock), goto end );
+                        CHECK_POSIX_DO( pthread_mutex_unlock(&order_lock), { ASSERT(0); } );
                         if (must_stop)
                                 goto end;

libfdcore/sctp.c

@@ -822 +822 @@
         
         /* Set the socket options */
-        CHECK_FCT_DO( ret = fd_setsockopt_prebind(*sock), goto fail );
+        CHECK_FCT_DO( ret = fd_setsockopt_prebind(*sock), goto out );
         
         /* Create the array of addresses, add first the configured addresses, then the discovered, then the other ones */
-        CHECK_FCT_DO( ret = add_addresses_from_list_mask(&sar.buf, &size, &count, family, htons(port), list, EP_FL_CONF,              EP_FL_CONF        ), goto fail );
-        CHECK_FCT_DO( ret = add_addresses_from_list_mask(&sar.buf, &size, &count, family, htons(port), list, EP_FL_CONF | EP_FL_DISC, EP_FL_DISC        ), goto fail );
-        CHECK_FCT_DO( ret = add_addresses_from_list_mask(&sar.buf, &size, &count, family, htons(port), list, EP_FL_CONF | EP_FL_DISC, 0         ), goto fail );
+        CHECK_FCT_DO( ret = add_addresses_from_list_mask(&sar.buf, &size, &count, family, htons(port), list, EP_FL_CONF,              EP_FL_CONF        ), goto out );
+        CHECK_FCT_DO( ret = add_addresses_from_list_mask(&sar.buf, &size, &count, family, htons(port), list, EP_FL_CONF | EP_FL_DISC, EP_FL_DISC        ), goto out );
+        CHECK_FCT_DO( ret = add_addresses_from_list_mask(&sar.buf, &size, &count, family, htons(port), list, EP_FL_CONF | EP_FL_DISC, 0         ), goto out );
         
         /* Try connecting */
@@ -866 +866 @@
                 /* Some errors are expected, we log at different level */
                 TRACE_DEBUG( lvl, "sctp_connectx returned an error: %s", strerror(ret));
-                goto fail;
+                goto out;
         }
         
@@ -872 +872 @@
         
         /* Set the remaining sockopts */
-        CHECK_FCT_DO( ret = fd_setsockopt_postbind(*sock, 1), goto fail_deco );
-        
-        /* Done! */
+        CHECK_FCT_DO( ret = fd_setsockopt_postbind(*sock, 1), 
+                { 
+                        CHECK_SYS_DO( shutdown(*sock, SHUT_RDWR), /* continue */ );
+                } );
+        
+out:
+        ;
         pthread_cleanup_pop(0);
-        return 0;
-        
-fail_deco:
-        CHECK_SYS_DO( shutdown(*sock, SHUT_RDWR), /* continue */ );
-fail:
-        if (*sock > 0) {
-                CHECK_SYS_DO( close(*sock), /* continue */ );
-                *sock = -1;
-        }
-        free(sar.buf);
+        
+        if (ret) {
+                if (*sock > 0) {
+                        CHECK_SYS_DO( close(*sock), /* continue */ );
+                        *sock = -1;
+                }
+                free(sar.buf);
+        }
         return ret;
 }

libfdcore/server.c

@@ -151 +151 @@
         pthread_cleanup_push((void *)fd_cnx_destroy, c->conn);
         pthread_cleanup_push((void *)fd_msg_free, msg);
-        CHECK_FCT_DO( fd_peer_handle_newCER( &msg, &c->conn ), goto cleanup );
+        CHECK_FCT_DO( fd_peer_handle_newCER( &msg, &c->conn ),  );
         pthread_cleanup_pop(0);
         pthread_cleanup_pop(0);

libfdcore/tcp.c

@@ -134 +134 @@
         CHECK_SYS(  s = socket(sa->sa_family, SOCK_STREAM, IPPROTO_TCP)  );
         
+        /* Set the socket options */
+        CHECK_FCT(  fd_tcp_setsockopt(sa->sa_family, s)  );
+        
         /* Cleanup if we are cancelled */
         pthread_cleanup_push(fd_cleanup_socket, &s);
-        
-        /* Set the socket options */
-        CHECK_FCT(  fd_tcp_setsockopt(sa->sa_family, s)  );
         
         TRACE_DEBUG_sSA(FULL, "Attempting TCP connection with peer: ", sa, NI_NUMERICHOST | NI_NUMERICSERV, "..." );