Mercurial > hg > freeDiameter

changeset 691:78b665400097

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Cleanup all pthread_cleanup_push / pop pairs so that pop is always called after push, or ASSERT(0) is some grave errors
author Sebastien Decugis <sdecugis@nict.go.jp>
date Thu, 20 Jan 2011 19:44:27 +0900
parents a29e4201d511
children dc1b3bd8ef54
files extensions/test_netemul/tne_process.c libfdcore/core.c libfdcore/p_cnx.c libfdcore/p_expiry.c libfdcore/p_out.c libfdcore/p_psm.c libfdcore/p_sr.c libfdcore/peers.c libfdcore/routing_dispatch.c libfdcore/sctp.c libfdcore/server.c libfdcore/tcp.c libfdproto/sessions.c
diffstat 13 files changed, 93 insertions(+), 79 deletions(-) [+]
line wrap: on
line diff
--- a/extensions/test_netemul/tne_process.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/extensions/test_netemul/tne_process.c	Thu Jan 20 19:44:27 2011 +0900
@@ -246,39 +246,35 @@
 	fd_log_threadname ( "test_netemul/process" );
 	
 	CHECK_POSIX_DO( pthread_mutex_lock(&mtx), goto error );
+	pthread_cleanup_push( fd_cleanup_mutex, &mtx );
 	
 	/* The loop */
 	while (1) {
 		/* First, test if we are canceled */
-		CHECK_POSIX_DO( pthread_mutex_unlock(&mtx), goto error );
 		pthread_testcancel();
-		CHECK_POSIX_DO( pthread_mutex_lock(&mtx), goto error );
-		
-		pthread_cleanup_push( fd_cleanup_mutex, &mtx );
 		
 		/* Send all messages that are ready (free resources before using new ones) */
-		CHECK_FCT_DO( send_all_ready(), goto error_ul );
+		CHECK_FCT_DO( send_all_ready(), break );
 		
 		/* Now process the new messages in input list for duplicate filter */
-		CHECK_FCT_DO( do_duplicates(), goto error_ul );
+		CHECK_FCT_DO( do_duplicates(), break );
 		
 		/* Now compute the latency for each new item */
-		CHECK_FCT_DO( do_latency(), goto error_ul );
+		CHECK_FCT_DO( do_latency(), break );
 		
 		/* Now, wait then loop */
 		if (FD_IS_LIST_EMPTY(&waitlist)) {
-			CHECK_POSIX_DO( pthread_cond_wait(&cnd, &mtx), goto error_ul );
+			CHECK_POSIX_DO( pthread_cond_wait(&cnd, &mtx), break );
 		} else {
 			CHECK_POSIX_DO2( pthread_cond_timedwait(&cnd, &mtx, &((struct process_item *)(waitlist.next))->ts), 
 				ETIMEDOUT, /* ETIMEDOUT is a normal return value, continue */,
-					/* on other error, */ goto error_ul );
+					/* on other error, */ break );
 		}
 		
-		pthread_cleanup_pop( 0 );
 		/* loop */
 	}
 
-error_ul:
+	pthread_cleanup_pop( 0 );
 	CHECK_POSIX_DO( pthread_mutex_unlock(&mtx),  );
 error:
 	TRACE_DEBUG(INFO, "A fatal error occurred in test_netemul/process thread!");
--- a/libfdcore/core.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/core.c	Thu Jan 20 19:44:27 2011 +0900
@@ -240,19 +240,21 @@
 /* For threads that would need to wait complete start of the framework (ex: in extensions) */
 int fd_core_waitstartcomplete(void)
 {
+	int ret = 0;
+	
 	TRACE_ENTRY("");
 	
 	CHECK_POSIX( pthread_mutex_lock( &is_ready_mtx ) );
 	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 ),  );
 	}
 	
 	pthread_cleanup_pop( 0 );
 	CHECK_POSIX( pthread_mutex_unlock( &is_ready_mtx ) );
 	
-	return 0;
+	return ret;
 }
 
 /* Start the server & client threads */
--- a/libfdcore/p_cnx.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/p_cnx.c	Thu Jan 20 19:44:27 2011 +0900
@@ -210,6 +210,7 @@
 	struct cnxctx * cnx = NULL;
 	struct next_conn * nc = NULL;
 	int rebuilt = 0;
+	int fatal_error=0;
 	
 	TRACE_ENTRY("%p", arg);
 	CHECK_PARAMS_DO( CHECK_PEER(peer), return NULL );
@@ -225,13 +226,13 @@
 		/* Rebuild the list if needed, if it is empty -- but at most once */
 		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 ++;
 			}
 			if (FD_IS_LIST_EMPTY(&peer->p_connparams)) {
 				/* 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;
 			}
 		}
@@ -276,27 +277,30 @@
 				fd_cnx_destroy(cnx);
 				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;
+out:
+	
+	if (fatal_error) {
 	
-fatal_error:
-	/* Cleanup the connection */
-	if (cnx)
-		fd_cnx_destroy(cnx);
+		/* 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), );
+		/* Generate a termination event */
+		CHECK_FCT_DO(fd_event_send(fd_g_config->cnf_main_ev, FDEV_TERMINATE, 0, NULL), );
+	}
 	
 	return NULL;
 }
--- a/libfdcore/p_expiry.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/p_expiry.c	Thu Jan 20 19:44:27 2011 +0900
@@ -97,7 +97,7 @@
 	fd_log_threadname ( "Peers/expire" );
 	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 );
 	
 	do {
@@ -107,7 +107,7 @@
 		/* Check if there are expiring sessions available */
 		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;
 		}
@@ -117,14 +117,14 @@
 		ASSERT( CHECK_PEER(first) );
 		
 		/* 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 */
 		if ( TS_IS_INFERIOR( &now, &first->p_exp_timer ) ) {
 			
 			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 */
 			continue;
@@ -132,14 +132,13 @@
 		
 		/* 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;
 }
--- a/libfdcore/p_out.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/p_out.c	Thu Jan 20 19:44:27 2011 +0900
@@ -68,13 +68,18 @@
 	
 	/* 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) */
 	if (*msg) {
 		CHECK_FCT( fd_msg_free(*msg) );
--- a/libfdcore/p_psm.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/p_psm.c	Thu Jan 20 19:44:27 2011 +0900
@@ -54,17 +54,18 @@
 /* Wait for start signal */
 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;
 }
 
 /* Allow the state machines to start */
--- a/libfdcore/p_sr.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/p_sr.c	Thu Jan 20 19:44:27 2011 +0900
@@ -116,7 +116,7 @@
 	return NULL;
 }
 
-/* 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;
 	struct msg * expired_req;
@@ -183,6 +183,7 @@
 error:	
 	; /* 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;
 }
 
--- a/libfdcore/peers.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/peers.c	Thu Jan 20 19:44:27 2011 +0900
@@ -532,8 +532,10 @@
 	for (v = validators.next; v != &validators; v = v->next) {
 		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;
 			goto out;
--- a/libfdcore/routing_dispatch.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/routing_dispatch.c	Thu Jan 20 19:44:27 2011 +0900
@@ -999,9 +999,9 @@
 		/* Test the current order */
 		{
 			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;
 			
--- a/libfdcore/sctp.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/sctp.c	Thu Jan 20 19:44:27 2011 +0900
@@ -821,12 +821,12 @@
 	pthread_cleanup_push(fd_cleanup_socket, sock);
 	
 	/* 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 */
 	if (TRACE_BOOL(FULL)) {
@@ -865,26 +865,28 @@
 		}
 		/* Some errors are expected, we log at different level */
 		TRACE_DEBUG( lvl, "sctp_connectx returned an error: %s", strerror(ret));
-		goto fail;
+		goto out;
 	}
 	
 	free(sar.buf); sar.buf = NULL;
 	
 	/* Set the remaining sockopts */
-	CHECK_FCT_DO( ret = fd_setsockopt_postbind(*sock, 1), goto fail_deco );
-	
-	/* Done! */
-	pthread_cleanup_pop(0);
-	return 0;
+	CHECK_FCT_DO( ret = fd_setsockopt_postbind(*sock, 1), 
+		{ 
+			CHECK_SYS_DO( shutdown(*sock, SHUT_RDWR), /* continue */ );
+		} );
 	
-fail_deco:
-	CHECK_SYS_DO( shutdown(*sock, SHUT_RDWR), /* continue */ );
-fail:
-	if (*sock > 0) {
-		CHECK_SYS_DO( close(*sock), /* continue */ );
-		*sock = -1;
+out:
+	;
+	pthread_cleanup_pop(0);
+	
+	if (ret) {
+		if (*sock > 0) {
+			CHECK_SYS_DO( close(*sock), /* continue */ );
+			*sock = -1;
+		}
+		free(sar.buf);
 	}
-	free(sar.buf);
 	return ret;
 }
 
--- a/libfdcore/server.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/server.c	Thu Jan 20 19:44:27 2011 +0900
@@ -150,7 +150,7 @@
 	/* Finally, pass the information to the peers module which will handle it next */
 	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);
 	
--- a/libfdcore/tcp.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdcore/tcp.c	Thu Jan 20 19:44:27 2011 +0900
@@ -133,12 +133,12 @@
 	/* Create the socket */
 	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, "..." );
 	
 	/* Try connecting to the remote address */
--- a/libfdproto/sessions.c	Thu Jan 20 15:38:12 2011 +0900
+++ b/libfdproto/sessions.c	Thu Jan 20 19:44:27 2011 +0900
@@ -175,7 +175,7 @@
 		/* Check if there are expiring sessions available */
 		if (FD_IS_LIST_EMPTY(&exp_sentinel)) {
 			/* Just wait for a change or cancelation */
-			CHECK_POSIX_DO( pthread_cond_wait( &exp_cond, &exp_lock ), break );
+			CHECK_POSIX_DO( pthread_cond_wait( &exp_cond, &exp_lock ), break /* this might not pop the cleanup handler, but since we ASSERT(0), it is not the big issue... */ );
 			/* Restart the loop on wakeup */
 			goto again;
 		}
@@ -206,7 +206,7 @@
 		
 	} while (1);
 	
-	TRACE_DEBUG(INFO, "An error occurred in session module! Expiry thread is terminating...");
+	TRACE_DEBUG(INFO, "A system error occurred in session module! Expiry thread is terminating...");
 	ASSERT(0);
 	return NULL;
 }
@@ -441,7 +441,7 @@
 
 		/* We added a new expiring element, we must signal */
 		if (li == &exp_sentinel) {
-			CHECK_POSIX( pthread_cond_signal(&exp_cond) );
+			CHECK_POSIX_DO( pthread_cond_signal(&exp_cond), { ASSERT(0); } ); /* if it fails, we might not pop the cleanup handlers, but this should not happen -- and we'd have a serious problem otherwise */
 		}
 		
 		#if 0
@@ -457,7 +457,7 @@
 		
 		/* We're done */
 		pthread_cleanup_pop(0);
-		CHECK_POSIX( pthread_mutex_unlock( &exp_lock ) );
+		CHECK_POSIX_DO( pthread_mutex_unlock( &exp_lock ), { ASSERT(0); } ); /* if it fails, we might not pop the cleanup handler, but this should not happen -- and we'd have a serious problem otherwise */
 	}
 	
 	pthread_cleanup_pop(0);
@@ -539,7 +539,7 @@
 
 	/* We added a new expiring element, we must signal if it was in first position */
 	if (session->expire.prev == &exp_sentinel) {
-		CHECK_POSIX( pthread_cond_signal(&exp_cond) );
+		CHECK_POSIX_DO( pthread_cond_signal(&exp_cond), { ASSERT(0); /* so that we don't have a pending cancellation handler */ } );
 	}
 
 	#if 0
@@ -572,15 +572,15 @@
 	*session = NULL;
 	
 	/* Unlink and invalidate */
-	CHECK_FCT( pthread_mutex_lock( H_LOCK(sess->hash) ) );
+	CHECK_POSIX( pthread_mutex_lock( H_LOCK(sess->hash) ) );
 	pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
-	CHECK_FCT( pthread_mutex_lock( &exp_lock ) );
+	CHECK_POSIX_DO( pthread_mutex_lock( &exp_lock ), { ASSERT(0); /* otherwise cleanup handler is not pop'd */ } );
 	fd_list_unlink( &sess->chain_h );
 	fd_list_unlink( &sess->expire ); /* no need to signal the condition here */
 	sess->eyec = 0xdead;
-	CHECK_FCT( pthread_mutex_unlock( &exp_lock ) );
+	CHECK_POSIX_DO( pthread_mutex_unlock( &exp_lock ), { ASSERT(0); /* otherwise cleanup handler is not pop'd */ } );
 	pthread_cleanup_pop(0);
-	CHECK_FCT( pthread_mutex_unlock( H_LOCK(sess->hash) ) );
+	CHECK_POSIX( pthread_mutex_unlock( H_LOCK(sess->hash) ) );
 	
 	/* Now destroy all states associated -- we don't take the lock since nobody can access this session anymore (in theory) */
 	while (!FD_IS_LIST_EMPTY(&sess->states)) {
@@ -613,7 +613,7 @@
 	
 	CHECK_POSIX( pthread_mutex_lock( H_LOCK(sess->hash) ) );
 	pthread_cleanup_push( fd_cleanup_mutex, H_LOCK(sess->hash) );
-	CHECK_POSIX( pthread_mutex_lock( &exp_lock ) );
+	CHECK_POSIX_DO( pthread_mutex_lock( &exp_lock ), { ASSERT(0); /* otherwise, cleanup not poped on FreeBSD */ } );
 	if (FD_IS_LIST_EMPTY(&sess->states)) {
 		fd_list_unlink( &sess->chain_h );
 		fd_list_unlink( &sess->expire );
@@ -621,7 +621,7 @@
 		free(sess->sid);
 		free(sess);
 	}
-	CHECK_POSIX( pthread_mutex_unlock( &exp_lock ) );
+	CHECK_POSIX_DO( pthread_mutex_unlock( &exp_lock ), { ASSERT(0); /* otherwise, cleanup not poped on FreeBSD */ } );
 	pthread_cleanup_pop(0);
 	CHECK_POSIX( pthread_mutex_unlock( H_LOCK(hash) ) );
 	
@@ -634,6 +634,7 @@
 	struct state *new;
 	struct fd_list * li;
 	int already = 0;
+	int ret = 0;
 	
 	TRACE_ENTRY("%p %p %p", handler, session, state);
 	CHECK_PARAMS( handler && VALIDATE_SH(handler) && session && VALIDATE_SI(session) && state );
@@ -643,7 +644,7 @@
 	pthread_cleanup_push( fd_cleanup_mutex, &session->stlock );
 			
 	/* Create the new state object */
-	CHECK_MALLOC(new = malloc(sizeof(struct state)) );
+	CHECK_MALLOC_DO(new = malloc(sizeof(struct state)), { ret = ENOMEM; goto out; } );
 	memset(new, 0, sizeof(struct state));
 	
 	new->eyec = SD_EYEC;
@@ -660,7 +661,7 @@
 		
 		if (st->hdl->id == handler->id) {
 			TRACE_DEBUG(INFO, "A state was already stored for session '%s' and handler '%p', at location %p", session->sid, st->hdl, st->state);
-			already = 1;
+			already = EALREADY;
 		}
 		
 		break;
@@ -672,11 +673,12 @@
 	} else {
 		free(new);
 	}
-	
+out:
+	;	
 	pthread_cleanup_pop(0);
 	CHECK_POSIX( pthread_mutex_unlock(&session->stlock) );
 	
-	return already ? EALREADY : 0;
+	return ret ?: already;
 }
 
 /* Get the data back */
"Welcome to our mercurial repository"