Mercurial > hg > waaad

changeset 289:6537213b55b5

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Simpified dispatch callbacks prototype and behavior
author Sebastien Decugis <sdecugis@nict.go.jp>
date Mon, 22 Dec 2008 12:29:49 +0900
parents beb9bde9a308
children 5ed9c73c31cd
files extensions/app_test/app_test.h extensions/app_test/atst_serv.c include/waaad/dispatch-api.h waaad/dispatch.c
diffstat 4 files changed, 87 insertions(+), 121 deletions(-) [+]
line wrap: on
line diff
--- a/extensions/app_test/app_test.h	Mon Dec 22 11:06:20 2008 +0900
+++ b/extensions/app_test/app_test.h	Mon Dec 22 12:29:49 2008 +0900
@@ -130,15 +130,6 @@
 	}										\
 }
 
-#define CHECK_NEG( _call ) {								\
-	int __ret = ( _call );								\
-	if ( __ret != 0 ) {								\
-		TRACE_DEBUG(INFO, "Error in function call");				\
-		log_error("Error in extension (" #_call "): %s\n", strerror(__ret));	\
-		return - __ret;								\
-	}										\
-}
-
 #define CHECK_VOID( _call ) {								\
 	int __ret = ( _call );								\
 	if ( __ret != 0 ) {								\
--- a/extensions/app_test/atst_serv.c	Mon Dec 22 11:06:20 2008 +0900
+++ b/extensions/app_test/atst_serv.c	Mon Dec 22 12:29:49 2008 +0900
@@ -41,25 +41,30 @@
 static disp_cb_hdl_t * hdl_tr = NULL; /* handler for Test-Request req cb */
 
 /* Default callback for the application. */
-static int fb_cb( msg_t * msg, msg_avp_t * avp, int handled )
+static int fb_cb( msg_t ** msg, msg_avp_t * avp )
 {
 	/* This CB should never be called */
-	TRACE_ENTRY("%p %p %d", msg, avp, handled);
+	TRACE_ENTRY("%p %p", msg, avp);
 	
 	log_error("Unexpected message received!\n");
 	
-	return -ENOTSUP;
+	return ENOTSUP;
 }
 
 /* Callback for incoming Test-Request messages */
-static int tr_cb( msg_t * msg, msg_avp_t * avp, int handled )
+static int tr_cb( msg_t ** msg, msg_avp_t * avp )
 {
-	msg_t * ans = msg;
+	msg_t * ans;
+	
+	TRACE_ENTRY("%p %p", msg, avp);
 	
-	TRACE_ENTRY("%p %p %d", msg, avp, handled);
+	if (msg == NULL)
+		return EINVAL;
+	
+	ans = *msg;
 	
 	/* Create answer header */
-	CHECK_NEG( msg_new_answ(&ans, MSGFL_FROM_TEMPLATE) );
+	CHECK( msg_new_answ(&ans, MSGFL_FROM_TEMPLATE) );
 	
 	/* Set the Session-Id AVP */
 	{
@@ -67,10 +72,10 @@
 		msg_avp_t * dst = NULL;
 		msg_avp_data_t * data = NULL;
 		
-		CHECK_NEG( msg_search_avp(msg, sess_id, &src) );
-		CHECK_NEG( msg_search_avp(ans, sess_id, &dst) );
-		CHECK_NEG( msg_avp_data( src, &data ) );
-		CHECK_NEG( msg_avp_setvalue( dst, data->avp_data) );
+		CHECK( msg_search_avp(*msg, sess_id, &src) );
+		CHECK( msg_search_avp(ans, sess_id, &dst) );
+		CHECK( msg_avp_data( src, &data ) );
+		CHECK( msg_avp_setvalue( dst, data->avp_data) );
 	}
 	/* Set the Test-AVP AVP */
 	{
@@ -78,10 +83,10 @@
 		msg_avp_t * dst = NULL;
 		msg_avp_data_t * data = NULL;
 		
-		CHECK_NEG( msg_search_avp(msg, atst_avp, &src) );
-		CHECK_NEG( msg_search_avp(ans, atst_avp, &dst) );
-		CHECK_NEG( msg_avp_data( src, &data ) );
-		CHECK_NEG( msg_avp_setvalue( dst, data->avp_data) );
+		CHECK( msg_search_avp(*msg, atst_avp, &src) );
+		CHECK( msg_search_avp(ans, atst_avp, &dst) );
+		CHECK( msg_avp_data( src, &data ) );
+		CHECK( msg_avp_setvalue( dst, data->avp_data) );
 	}
 	
 	/* Set the Origin-Host AVP */
@@ -92,8 +97,8 @@
 		data.os.data = (unsigned char *)(g_pconf->diameter_identity);
 		data.os.len  = strlen(g_pconf->diameter_identity);
 		
-		CHECK_NEG( msg_search_avp(ans, origin_host, &dst) );
-		CHECK_NEG( msg_avp_setvalue( dst, &data) );
+		CHECK( msg_search_avp(ans, origin_host, &dst) );
+		CHECK( msg_avp_setvalue( dst, &data) );
 	}
 	
 	/* Set the Origin-Realm AVP */
@@ -104,8 +109,8 @@
 		data.os.data = (unsigned char *)(g_pconf->diameter_realm);
 		data.os.len  = strlen(g_pconf->diameter_realm);
 		
-		CHECK_NEG( msg_search_avp(ans, origin_realm, &dst) );
-		CHECK_NEG( msg_avp_setvalue( dst, &data) );
+		CHECK( msg_search_avp(ans, origin_realm, &dst) );
+		CHECK( msg_avp_setvalue( dst, &data) );
 	}
 	
 	/* Set the Result-Code AVP */
@@ -115,14 +120,15 @@
 		
 		data.u32 = 2001; /* Success */
 		
-		CHECK_NEG( msg_search_avp(ans, res_code, &dst) );
-		CHECK_NEG( msg_avp_setvalue( dst, &data) );
+		CHECK( msg_search_avp(ans, res_code, &dst) );
+		CHECK( msg_avp_setvalue( dst, &data) );
 	}
 	
 	/* Send the answer */
-	CHECK_NEG( msg_send( &ans, NULL, NULL ) );
+	CHECK( msg_send( &ans, NULL, NULL ) );
 	
-	return DISP_CBRET_HANDLED_STOP;
+	*msg = NULL;
+	return 0;
 }
 
 int serv_init(void)
--- a/include/waaad/dispatch-api.h	Mon Dec 22 11:06:20 2008 +0900
+++ b/include/waaad/dispatch-api.h	Mon Dec 22 12:29:49 2008 +0900
@@ -69,13 +69,12 @@
  * When a callback is called, it receives the message as parameter, and eventually a pointer to 
  * the AVP in the message when this is appropriate.
  *
- * The callback must process the message, and then return a value with the following meaning:
- *  (negative): -errno, if an error occurred.
- *  (positive): one of the disp_cb_ret_t values. See the definition for more information.
+ * The callback must process the message, and eventually create an answer to it. See the definition
+ * bellow for more information.
  *
- * At least one of the registered callbacks must return a value of DIST_CBRET_HANDLED_* on the message, or
- * a default handler will be called with the effect of requeuing the message for forwarding on the network to 
- * another peer (for requests), or discarding the message (for answers).
+ * If no callback has handled the message, a default handler will be called with the effect of
+ * requeuing the message for forwarding on the network to another peer (for requests), or 
+ * discarding the message (for answers).
  */
 
 #ifndef _DISPATCH_API_H
@@ -83,17 +82,6 @@
 
 #include <waaad/message-api.h>
 
-/* The values that must be returned by a callback, when no error occurred: */
-typedef enum {
-	DISP_CBRET_CONTINUE = 0,	/* The callback has completed its process without error. 
-					   Next callback can be called.
-					   This value is typically returned by non-final callbacks (for example cb registered on AVPs) */
-	DISP_CBRET_HANDLED_CONTINUE,	/* The callback has processed the message (for example answered a request or 
-					   taken action from an answer). Remaining callbacks may be called, if any. */
-	DISP_CBRET_HANDLED_STOP		/* The callback has processed the message (for example generated an error answer or forwarded the 
-					   message to another peer). No additional handler should be called on this message. */
-} disp_cb_ret_t;
-
 /* The allowed methods for registering a callback. */
 typedef enum {
 	DISP_REG_ANY = 1,		/* The callback is called on any message. This should be only used for debug. */
@@ -130,7 +118,7 @@
  * DISP_REG_ANY.
  *  In this case, "when" must be NULL.
  *
- * In any other case (yet), the "when" parameter points to a disp_reg_val_t structure. Detail is given bellow.
+ * In any other case, the "when" parameter points to a valid disp_reg_val_t structure. Detail is given bellow.
  *
  * DISP_REG_APPID.
  *  Only the "app_id" field must be set, other fields are ignored. It points to a dictionary object of type DICT_APPLICATION.
@@ -165,21 +153,21 @@
  * CALLBACK:	disp_cb_t
  *
  * PARAMETERS:
- *  msg     : The message that trigged the callback call.
+ *  msg     : The location of message that trigged the callback call.
  *  avp     : for callbacks registered with DISP_REG_AVP or DISP_REG_AVP_ENUMVAL, this points to the triggering AVP. NULL otherwise.
- *  handled : (boolean) a previous handler has returned DISP_CBRET_HANDLED_CONTINUE already?
  *
  * DESCRIPTION: 
  *   This is the prototype of the callback functions that are registered with dicp_cb_reg function.
  *  See introduction and previous comments for more information on how this callback is called.
- *
+ *  
+ *  The callback must set *msg to NULL if it creates an answer or "handles" a received answer, so that no further processing is done.
+ * 
  * RETURN VALUE:
- *  < 0 			: An error occurred (ex: -EINVAL = invalid parameter; -ENOMEM: not enough memory, ...).
- *  DISP_CBRET_CONTINUE		: The next callback can be called, no action has been taken yet.
- *  DISP_CBRET_HANDLED_CONTINUE : Action has been taken, next callback can be called (used in normal condition)
- *  DISP_CBRET_HANDLED_STOP	: Action has been taken, no more callback must be called (used in error conditions)
+ *   0	: success. 
+ *  !0 	: An error occurred (ex: EINVAL = invalid parameter; ENOMEM: not enough memory, ...).
+ *	 In this case, the error is logged in the daemon and the message is discarded.
  */
-typedef int (*disp_cb_t) ( msg_t * msg, msg_avp_t * avp, int handled );
+typedef int (*disp_cb_t) ( msg_t ** msg, msg_avp_t * avp );
 
 /* The following opaque type represents a handler to a registered callback. This allows to remove a registered callback. */
 typedef void disp_cb_hdl_t;
@@ -206,7 +194,7 @@
  *  EINVAL 	: A parameter is invalid.
  *  ENOMEM	: Not enough memory to complete the operation
  */
-int disp_register ( disp_cb_t cb, disp_reg_t how, void * when, disp_cb_hdl_t ** handle );
+int disp_register ( disp_cb_t cb, disp_reg_t how, disp_reg_val_t * when, disp_cb_hdl_t ** handle );
 
 
 /*
@@ -238,7 +226,7 @@
 	int	version;	/* The version of this API/ABI, must be WAAAD_API_DISP_VER */
 	
 	/* the remaining is dispatching-specific */
-	int (*disp_register)    ( disp_cb_t cb, disp_reg_t how, void * when, disp_cb_hdl_t ** handle );
+	int (*disp_register)    ( disp_cb_t cb, disp_reg_t how, disp_reg_val_t * when, disp_cb_hdl_t ** handle );
 	int (*disp_unregister)  ( disp_cb_hdl_t * handle );
 } api_disp_t;
 
--- a/waaad/dispatch.c	Mon Dec 22 11:06:20 2008 +0900
+++ b/waaad/dispatch.c	Mon Dec 22 12:29:49 2008 +0900
@@ -41,7 +41,7 @@
 #include "waaad-internal.h"
 
 /* Read-write lock for dictionary callbacks. Note that if the dictionary object is destroyed, this lock is not taken. */
-static pthread_rwlock_t	dict_lock;
+static pthread_rwlock_t	disp_dictlock;
 
 /* Keep the list of all registered handlers */
 static uti_list_t all_handlers;
@@ -83,12 +83,12 @@
 	CHECK_FCT(  msg_data(*msg, &hdr)  );
 	
 	/* Now we search the application of the message */
-	CHECK_FCT_DO( dict_search( DICT_APPLICATION, APPLICATION_BY_ID_REF, &hdr->msg_appl, app ),
-		{
-			/* In case of error, we should answer a DIAMETER_APPLICATION_UNSUPPORTED error */
-			TRACE_DEBUG(INFO, "Error: TODO reply DIAMETER_APPLICATION_UNSUPPORTED if it's a request");
-			goto end;
-		} );
+	CHECK_FCT( dict_search( DICT_APPLICATION, APPLICATION_BY_ID_REF, &hdr->msg_appl, app ) );
+	if (*app == NULL) {
+		/* In case of error, we should answer a DIAMETER_APPLICATION_UNSUPPORTED error */
+		TRACE_DEBUG(INFO, "Error: TODO reply DIAMETER_APPLICATION_UNSUPPORTED if it's a request");
+		goto end;
+	}
 		
 	CHECK_FCT_DO( ret = msg_parse_rules( *msg, &rule ), /* handle error later, but log here */ );
 	
@@ -110,33 +110,17 @@
 	return ret;
 }
 
-#define loc_CALL_CB( _cb, _msg, _avp ) {								\
-	int _ret = 0;											\
+/**************************************************************************************/
+
+#define loc_CALL_CB( _cb, _pmsg, _avp ) {								\
 	TRACE_DEBUG(ANNOYING, "Calling dispatch callback...");						\
-	_ret = (*(_cb))( (_msg), (_avp), hdld );							\
-	if (_ret < 0) {											\
-		TRACE_DEBUG(INFO, "Callback returned an error: %s\n, continuing...", strerror(-_ret));	\
-	} else {											\
-		int _must_stop = 0;									\
-		switch (_ret) {										\
-			case DISP_CBRET_HANDLED_STOP:							\
-				_must_stop = 1;								\
-			case DISP_CBRET_HANDLED_CONTINUE:						\
-				hdld += 1;								\
-			case DISP_CBRET_CONTINUE:							\
-				TRACE_DEBUG(ANNOYING, "Dispatch callback returned");			\
-				break;									\
-			default:									\
-			TRACE_DEBUG(INFO, "Dispatch callback returned unknown value (%d)", _ret);	\
-		}											\
-		if (_must_stop)										\
-			goto c_continue;								\
-	}												\
+	CHECK_FCT_DO((*(_cb))( (_pmsg), (_avp) ), goto c_continue);					\
+	TRACE_DEBUG(ANNOYING, "Dispatch callback returned, %p", *(_pmsg));				\
+	if (*(_pmsg) == NULL)										\
+		goto c_continue;									\
 }
 		
 
-/**************************************************************************************/
-
 /* Dispatch thread */
 static void * disp_th(void * arg)
 {
@@ -151,7 +135,6 @@
 		uti_list_t * li = NULL;
 		uti_list_t * sem = NULL;
 		_disp_cb_hdl_t * hdl = NULL;
-		int hdld = 0;
 		dict_object_t * msg_app = NULL;
 		dict_object_t * msg_cmd = NULL;
 				
@@ -182,7 +165,7 @@
 				(*anscb)(data, &msg);
 				
 				if (msg == NULL) {
-					TRACE_DEBUG(FULL, "The message was handled by msg_send callback, not calling regular callbacks");
+					TRACE_DEBUG(FULL, "The message was handled by msg_send callback, skipping dispatch module callbacks");
 					continue;
 				}
 			}
@@ -194,13 +177,13 @@
 			continue;
 		
 		/* Now we'll call the dispatch handlers, lock the appropriate rdlock */
-		CHECK_POSIX_DO( pthread_rwlock_rdlock(&dict_lock), goto error );
-		pthread_cleanup_push( (void *)pthread_rwlock_unlock, &dict_lock );
+		CHECK_POSIX_DO( pthread_rwlock_rdlock(&disp_dictlock), goto error );
+		pthread_cleanup_push( (void *)pthread_rwlock_unlock, &disp_dictlock );
 		
 		/* Start with calling all ANY callbacks */
-		for (li = any_handlers.next; li != &any_handlers; li = li->next) {
+		for (li = any_handlers.next; (li != &any_handlers); li = li->next) {
 			hdl = _H(li->o);
-			loc_CALL_CB( hdl->cb, msg, NULL );
+			loc_CALL_CB( hdl->cb, &msg, NULL );
 		}
 		
 		/* Now get the reference to the message dict object */
@@ -250,7 +233,7 @@
 						continue;
 					
 					/* Ok, this callback should be called */
-					loc_CALL_CB( hdl->cb, msg, NULL );
+					loc_CALL_CB( hdl->cb, &msg, avp );
 				}
 			}
 			/* Go to next AVP */
@@ -268,7 +251,7 @@
 				continue;
 
 			/* Ok, this callback should be called */
-			loc_CALL_CB( hdl->cb, msg, NULL );
+			loc_CALL_CB( hdl->cb, &msg, NULL );
 		}
 		
 		/* Finally call the application's callbacks */
@@ -278,16 +261,14 @@
 
 			ASSERT( hdl->when.app_id == msg_app );
 
-			loc_CALL_CB( hdl->cb, msg, NULL );
+			loc_CALL_CB( hdl->cb, &msg, NULL );
 		}
 		
-		/* We're done; now check the message has been handled at least once */
-		if (hdld == 0) {
-			TRACE_DEBUG(INFO, "Error: the message was not handled in dispatch module...");
+		/* If we arrive here, it means no callback has set msg to NULL... */
+		TRACE_DEBUG(INFO, "Error: the message was not handled in dispatch module...TODO");
 			
-			/* TODO: request => forward or return error? answer => discard */
-			
-		}
+		/* TODO: request => forward or return error? answer => discard */
+	
 		
 		goto c_continue;
 c_error:
@@ -295,7 +276,7 @@
 c_continue:
 		/* some compilers complain if there is no instruction here... => */ ;
 		pthread_cleanup_pop( 0 );
-		CHECK_POSIX_DO( pthread_rwlock_unlock(&dict_lock), goto error );
+		CHECK_POSIX_DO( pthread_rwlock_unlock(&disp_dictlock), goto error );
 		
 		if (msg) {
 			CHECK_FCT_DO( msg_free(msg, 1), goto error );
@@ -320,7 +301,7 @@
 	
 	uti_list_init(&all_handlers, NULL);
 	uti_list_init(&any_handlers, NULL);
-	CHECK_POSIX( pthread_rwlock_init(&dict_lock, NULL) );
+	CHECK_POSIX( pthread_rwlock_init(&disp_dictlock, NULL) );
 	
 	CHECK_POSIX(  pthread_create( &th, NULL, disp_th, NULL )  );
 	
@@ -340,13 +321,13 @@
 		disp_unregister((disp_cb_hdl_t *)(all_handlers.next));
 	}
 	
-	CHECK_POSIX_DO( pthread_rwlock_destroy(&dict_lock), /* continue */ );
+	CHECK_POSIX_DO( pthread_rwlock_destroy(&disp_dictlock), /* continue */ );
 	
 	return 0;
 }
 
 /* Register a new callback. */
-int disp_register ( disp_cb_t cb, disp_reg_t how, void * when, disp_cb_hdl_t ** handle )
+int disp_register ( disp_cb_t cb, disp_reg_t how, disp_reg_val_t * when, disp_cb_hdl_t ** handle )
 {
 	_disp_cb_hdl_t * hdl = NULL;
 	uti_list_t * where = NULL;
@@ -356,7 +337,7 @@
 	CHECK_PARAMS( cb );
 	CHECK_PARAMS( (how >= DISP_REG_ANY) && (how <= DISP_REG_AVP_ENUMVAL) );
 	CHECK_PARAMS( (how == DISP_REG_ANY) || (when != NULL) );
-	CHECK_PARAMS( (how != DISP_REG_APPID) || (((disp_reg_val_t *)when)->flags & 0x3) ); 
+	CHECK_PARAMS( (how != DISP_REG_APPID) || (when->flags & 0x3) ); 
 	
 	switch (how) {
 		case DISP_REG_ANY:
@@ -364,16 +345,16 @@
 			break;
 	
 		case DISP_REG_APPID:
-			CHECK_FCT( dict_disp_cb(DICT_APPLICATION, ((disp_reg_val_t *)when)->app_id, &where) );
+			CHECK_FCT( dict_disp_cb(DICT_APPLICATION, when->app_id, &where) );
 			break;
 			
 		case DISP_REG_CC:
-			CHECK_FCT( dict_disp_cb(DICT_COMMAND, ((disp_reg_val_t *)when)->command, &where) );
+			CHECK_FCT( dict_disp_cb(DICT_COMMAND, when->command, &where) );
 			break;
 			
 		case DISP_REG_AVP:
 		case DISP_REG_AVP_ENUMVAL:
-			CHECK_FCT( dict_disp_cb(DICT_AVP, ((disp_reg_val_t *)when)->avp, &where) );
+			CHECK_FCT( dict_disp_cb(DICT_AVP, when->avp, &where) );
 			break;
 	}
 	
@@ -388,12 +369,12 @@
 		memcpy(&hdl->when, when, sizeof(disp_reg_val_t));
 	hdl->cb = cb;
 	
-	CHECK_POSIX( pthread_rwlock_wrlock(&dict_lock) );
+	CHECK_POSIX( pthread_rwlock_wrlock(&disp_dictlock) );
 	
 	uti_list_insert_before(&all_handlers, &hdl->all);
 	uti_list_insert_before(where, &hdl->dict);
 	
-	CHECK_POSIX( pthread_rwlock_unlock(&dict_lock) );
+	CHECK_POSIX( pthread_rwlock_unlock(&disp_dictlock) );
 	
 	if (handle)
 		*handle = (disp_cb_hdl_t *) hdl;
@@ -407,14 +388,14 @@
 		dict_object_t * vendor = NULL;
 		dict_vendor_data_t vendor_data;
 		
-		add_auth = (((disp_reg_val_t *)when)->flags & 0x1) ? 1 : 0;
-		add_acct = (((disp_reg_val_t *)when)->flags & 0x2) ? 1 : 0;
+		add_auth = (when->flags & 0x1) ? 1 : 0;
+		add_acct = (when->flags & 0x2) ? 1 : 0;
 		
 		/* Get the app id of the application being added */
-		CHECK_FCT( dict_getval( ((disp_reg_val_t *)when)->app_id, &appl_data) );
+		CHECK_FCT( dict_getval( when->app_id, &appl_data) );
 		
 		/* Get the vendor and data */
-		CHECK_FCT( dict_search( DICT_VENDOR, VENDOR_OF_APPLICATION, ((disp_reg_val_t *)when)->app_id, &vendor) );
+		CHECK_FCT( dict_search( DICT_VENDOR, VENDOR_OF_APPLICATION, when->app_id, &vendor) );
 		if (vendor) {
 			CHECK_FCT( dict_getval( vendor, &vendor_data) );
 		}
@@ -470,10 +451,10 @@
 	
 	CHECK_PARAMS( CHECK_HDL(handle) );
 	
-	CHECK_POSIX( pthread_rwlock_wrlock(&dict_lock) );
+	CHECK_POSIX( pthread_rwlock_wrlock(&disp_dictlock) );
 	uti_list_unlink(&_H(handle)->all);
 	uti_list_unlink(&_H(handle)->dict);
-	CHECK_POSIX( pthread_rwlock_unlock(&dict_lock) );
+	CHECK_POSIX( pthread_rwlock_unlock(&disp_dictlock) );
 	
 	free(handle);
"Welcome to our mercurial repository"