drouting/qrouting: Improve sorting code

    * fix badly handled and non-handled error cases
    * fix misleading comments
    * remove solved TODOs
    * normalize callback names and coding style
    * fix typos

modules/drouting/dr_api_internal.c

@@ -53,7 +53,7 @@ static int get_cr_n_gw(pcr_t * cr) {
 }
 
 static inline pgw_t * get_gw_from_cr(pcr_t *cr, int n) {
-	if(cr->pgwa_len > n)
+	if (cr->pgwa_len > n)
 		return cr->pgwl[n].dst.gw; /* a carrier cannot contain another carrier */
 	return NULL; /* provided index was bigger than the vector */
 }

modules/drouting/dr_cb.c

@@ -164,10 +164,11 @@ int register_dr_cb(enum drcb_types type, dr_cb f, void *param,
  * the index within the array */
 int run_dr_sort_cbs(sort_cb_type type, struct dr_sort_params *param)
 {
-	if(dr_sort_cbs[type] == NULL) {
+	if (!dr_sort_cbs[type]) {
 		LM_WARN("callback type '%d' not registered\n", type);
 		return -1;
 	}
+
 	dr_sort_cbs[type]->callback(param);
 	return 0;
 }

modules/drouting/dr_cb.h

@@ -62,7 +62,6 @@ struct dr_callback {
 	dr_param_free_cb callback_param_free;
 	struct dr_callback *next;
 };
-/* TODO: should i have a different structure for sort_callbacks */
 
 int register_dr_cb(enum drcb_types type, dr_cb f, void *param,
 		dr_param_free_cb ff);

modules/drouting/dr_cb_sorting.h

@@ -67,10 +67,15 @@ struct dr_acc_call_params {
 
 struct dr_sort_params {
 	rt_info_t *dr_rule; /* dr_rule which contains the dst to be sorted */
-	unsigned short dst_id; /* the size of pgwl */
-	unsigned short *sorted_dst; /* returns an array with the
-								 * indexes of the sorted dest */
-	int rc; /* return code for the funciton */
+
+	/* -1 for rule gwlist sort, carrier dst index for carrier gwlist sort */
+	unsigned short dst_idx;
+
+	/* output (pre-allocated): sorted array of dst indexes */
+	unsigned short *sorted_dst;
+
+	/* output: return code of the callback (0: success) */
+	int rc;
 };
 
 struct dr_add_rule_params {

modules/drouting/drouting.c

@@ -334,11 +334,11 @@ mi_response_t *mi_dr_enable_probing_1(const mi_params_t *params,
 unsigned int *dr_enable_probing_state=0;
 
 /* sorting functions used by dr */
-static void no_sort(void *params);
+static void no_sort_cb(void *params);
 static void weight_based_sort_cb(void *params);
 static int weight_based_sort(pgw_list_t *pgwl, int size, unsigned short *idx);
-static int sort_rt_dst(rt_info_t *dr_rule, unsigned short dst_id, unsigned short *idx);
-static inline int get_pgwl_params(struct dr_sort_params *sort_params,
+static int sort_rt_dst(rt_info_t *dr_rule, unsigned short dst_idx, unsigned short *idx);
+static inline int get_pgwl_params(struct dr_sort_params *dsp,
 		pgw_list_t **pgwl, int *size, unsigned short **sorted_dst);
 
 
@@ -1448,16 +1448,15 @@ static int dr_init(void)
 	}
 
 	/* register dr callbacks for sorting */
-	if(register_dr_cb(DRCB_SORT_DST, no_sort, (void*)NO_SORT, NULL) < 0) {
-		LM_ERR("[DR] failed to register DRCB_SORT_DST callback [no_sort] to dr\n");
+	if (register_dr_cb(DRCB_SORT_DST, no_sort_cb, (void *)NO_SORT, NULL) < 0) {
+		LM_ERR("failed to register no_sort cb\n");
 		return -1;
 	}
-	if(register_dr_cb(DRCB_SORT_DST, weight_based_sort_cb,
-				(void*)WEIGHT_BASED_SORT, NULL) < 0) {
-		LM_ERR("[DR] failed to register DRCB_SORT_DST callback" \
-				" [weight_based_sort] to dr\n");
-		return -1;
 
+	if (register_dr_cb(DRCB_SORT_DST, weight_based_sort_cb,
+				(void *)WEIGHT_BASED_SORT, NULL) < 0) {
+		LM_ERR("failed to register weight_based_sort cb\n");
+		return -1;
 	}
 
 	name_w_part.s = shm_malloc( MAX_LEN_NAME_W_PART /* length of
@@ -2262,7 +2261,6 @@ static int use_next_gw(struct sip_msg* msg,
 	int ok = 0;
 	pgw_t * dst;
 	struct socket_info *sock;
-	struct dr_acc_call_params *acc_call_params;
 
 	if(part==NULL) {
 		LM_ERR("Partition is mandatory for use_next_gw.\n");
@@ -2407,15 +2405,13 @@ static int use_next_gw(struct sip_msg* msg,
 			if (avp_sk) destroy_avp(avp_sk);
 			avp_sk = search_first_avp( 0, current_partition->acc_call_params_avp,
 					&val, NULL);
-		}while (avp_sk && (avp_sk->flags&AVP_VAL_STR)==0 );
+		} while (avp_sk && !(avp_sk->flags & AVP_VAL_STR));
+
 		if (!avp_sk) {
 			/* this shuold not happen, it is a bogus state */
-			acc_call_params = NULL;
+			LM_BUG("call params AVP not found\n");
 		} else {
-			acc_call_params = (struct dr_acc_call_params*)
-				(*(struct dr_acc_call_params**)val.s.s);
-			run_dr_cbs(DRCB_ACC_CALL, acc_call_params); /* do accouting
-																		  for the call */
+			run_dr_cbs(DRCB_ACC_CALL, *(struct dr_acc_call_params **)val.s.s);
 			destroy_avp(avp_sk);
 		}
 
@@ -2506,105 +2502,112 @@ static int use_next_gw(struct sip_msg* msg,
 		} \
 	}while(0) \
 
-/* don't sort anything let the list as it is */
-static void no_sort(void *params) {
-	struct dr_sort_params *dsp = (struct dr_sort_params *)params;
-	int i = 0;
-	unsigned short *sorted_dst = NULL;
-	int size = 0;
-	pgw_list_t *pgwl = NULL;
-	int rc = 0;
-
-	rc = get_pgwl_params(dsp, &pgwl, &size, &sorted_dst);
-
-
-	for(i = 0; i < size; i++) {
-		sorted_dst[i] = i; /* leave the gw list as itis */
-	}
-
-	if(rc < 0) {
-		LM_ERR("failed to sort\n");
-		dsp->rc = -1; /* everything ok */
-	}
-	dsp->sorted_dst = sorted_dst;
-	dsp->rc = 0; /* everything ok */
-}
-static inline int get_pgwl_params(struct dr_sort_params *sort_params,
-		pgw_list_t **pgwl, int *size, unsigned short **sorted_dst) {
-	if(sort_params->dst_id == (unsigned short)-1) {
-		*pgwl = sort_params->dr_rule->pgwl;
-		*size = sort_params->dr_rule->pgwa_len;
+static inline int get_pgwl_params(struct dr_sort_params *dsp,
+		pgw_list_t **pgwl, int *size, unsigned short **sorted_dst)
+{
+	if (dsp->dst_idx == (unsigned short)-1) {
+		*pgwl = dsp->dr_rule->pgwl;
+		*size = dsp->dr_rule->pgwa_len;
 	} else { /* it is a carrier */
-		if(sort_params->dst_id >= 0 && sort_params->dst_id < sort_params->dr_rule->pgwa_len) {
-			if(sort_params->dr_rule->pgwl[sort_params->dst_id].is_carrier) {
-				*pgwl = sort_params->dr_rule->pgwl[sort_params->dst_id].dst.carrier->pgwl;
-				*size = sort_params->dr_rule->pgwl[sort_params->dst_id].dst.carrier->pgwa_len;
+		if (dsp->dst_idx >= 0 && dsp->dst_idx < dsp->dr_rule->pgwa_len) {
+			if (dsp->dr_rule->pgwl[dsp->dst_idx].is_carrier) {
+				*pgwl = dsp->dr_rule->pgwl[dsp->dst_idx].dst.carrier->pgwl;
+				*size = dsp->dr_rule->pgwl[dsp->dst_idx].dst.carrier->pgwa_len;
 			} else {
 				LM_WARN("provided destination for sorting is not a carrier\n");
 				return -1;
 			}
 		} else {
-			LM_WARN("no destination with this id (%d)\n", sort_params->dst_id);
+			LM_WARN("no destination with this id (%d)\n", dsp->dst_idx);
 			return -1;
 		}
 	}
-	*sorted_dst = sort_params->sorted_dst;
+
+	*sorted_dst = dsp->sorted_dst;
 	return 0;
 }
 
 #define DR_MAX_GWLIST	64
 
-static int sort_rt_dst(rt_info_t *dr_rule, unsigned short dst_id,
+static int sort_rt_dst(rt_info_t *dr_rule, unsigned short dst_idx,
 		unsigned short *idx)
 {
-	struct dr_sort_params sort_params;
-	pgw_list_t * pgwl;
+	struct dr_sort_params dsp;
+	pgw_list_t *_;
 	int i;
 	int size;
-	unsigned short *tmp;
+	unsigned short *__;
 	unsigned char sort_alg;
 
-	memset(&sort_params, 0, sizeof sort_params);
-	sort_params.dr_rule = dr_rule;
-	sort_params.dst_id = dst_id;
-	sort_params.sorted_dst = idx;
+	memset(&dsp, 0, sizeof dsp);
+	dsp.dr_rule = dr_rule;
+	dsp.dst_idx = dst_idx;
+	dsp.sorted_dst = idx;
 
-	if (get_pgwl_params(&sort_params, &pgwl, &size, &tmp) < 0)
+	if (get_pgwl_params(&dsp, &_, &size, &__) < 0) {
+		LM_ERR("failed to extract params\n");
 		return -1;
+	}
 
 	/* extract the sorting algorithm */
-	if(dst_id != (unsigned short)-1) { /* if destionation is carrier */
-		sort_alg = dr_rule->pgwl[dst_id].dst.carrier->sort_alg;
-	} else { /* if destionation is gw */
+	if (dst_idx == (unsigned short)-1) /* destination is a gw */
 		sort_alg = dr_rule->sort_alg;
-	}
+	else /* destination is a carrier */
+		sort_alg = dr_rule->pgwl[dst_idx].dst.carrier->sort_alg;
 
-	run_dr_sort_cbs(sort_alg, &sort_params);
+	run_dr_sort_cbs(sort_alg, &dsp);
+	if (dsp.rc != 0) {
+		LM_ERR("failed to sort destinations (%d)\n", dsp.rc);
+		return -1;
+	}
 
 	LM_DBG("Sorted destination list:\n");
-	for(i = 0; i < size; i++) {
-		LM_DBG("%d\n",idx[i]);
-	}
+	for (i = 0; i < size; i++)
+		LM_DBG("%d\n", idx[i]);
 
 	return 0;
 }
+
+/* preserve the order of the destinations */
+static void no_sort_cb(void *params)
+{
+	struct dr_sort_params *dsp = (struct dr_sort_params *)params;
+	int i = 0;
+	unsigned short *sorted_dst = NULL;
+	int size = 0;
+	pgw_list_t *pgwl = NULL;
+	int rc = 0;
+
+	rc = get_pgwl_params(dsp, &pgwl, &size, &sorted_dst);
+	if (rc < 0) {
+		LM_ERR("failed to sort\n");
+		dsp->rc = -1;
+		return;
+	}
+
+	for (i = 0; i < size; i++)
+		sorted_dst[i] = i;
+
+	dsp->rc = 0; /* everything ok */
+}
+
 /* sort based on the weight of the gws */
 static void weight_based_sort_cb(void *params)
 {
 	pgw_list_t *pgwl;
 	int size;
 	int rc;
-	unsigned short *idx;
+	unsigned short *sorted_dst;
 	struct dr_sort_params *dsp = (struct dr_sort_params *)params;
 
-	rc = get_pgwl_params(dsp, &pgwl, &size, &idx);
-	if(rc < 0) {
+	rc = get_pgwl_params(dsp, &pgwl, &size, &sorted_dst);
+	if (rc < 0) {
 		LM_WARN("failed to sort\n");
 		dsp->rc = -1;
-		return ;
+		return;
 	}
 
-	if (weight_based_sort(pgwl, size, idx) < 0)
+	if (weight_based_sort(pgwl, size, sorted_dst) < 0)
 		dsp->rc = -1;
 	else
 		dsp->rc = 0;
@@ -3125,7 +3128,7 @@ static int do_routing(struct sip_msg* msg, struct head_db *part, int grp,
 		whitelist->s[whitelist->len] = tmp;
 	}
 
-	/* iterate through the list, skip the disabled destination */
+	/* walk the sorted list, skip disabled destinations */
 	for ( i=0 ; i<rt_info->pgwa_len ; i++ ) {
 
 		if(dsts_idx[i] == (unsigned short)-1) {
@@ -3162,8 +3165,8 @@ static int do_routing(struct sip_msg* msg, struct head_db *part, int grp,
 			for ( j=0 ; j<dst->dst.carrier->pgwa_len ; j++ ) {
 
 				if(carrier_idx[j] == (unsigned short)-1) {
-					LM_DBG("All available destinations (for carrier id '%d') were"\
-							"inserted\n", carrier_idx[j]);
+					LM_DBG("All available destinations (dst idx %d) were"
+					       "inserted\n", carrier_idx[j]);
 					break;
 				}
 

modules/drouting/prefix_tree.h

@@ -136,7 +136,7 @@ typedef struct rt_info_ {
 	unsigned short pgwa_len;
 	/* how many lists link this element */
 	unsigned short ref_cnt;
-	/* handler used by qr for accouting (actually qr_rule_t*) */
+	/* handler used by qr for accounting (actually qr_rule_t *) */
 	void *qr_handler;
 	/* sorting algorithm for the destinations inside rule */
 	unsigned char sort_alg;

modules/qrouting/qr_acc.c

@@ -230,7 +230,7 @@ void qr_check_reply_tmcb(struct cell *cell, int type, struct tmcb_params *ps)
 {
 	double pdd_tm = 0, st = 0;
 	qr_trans_prop_t *trans_prop = (qr_trans_prop_t *)*ps->param;
-	struct dlg_cell *cur_dlg; /* for accouting call time */
+	struct dlg_cell *cur_dlg; /* for accounting call time */
 	struct qr_dialog_prop *dialog_prop;
 
 	if (ps->code == 180 || ps->code == 183) { /* Ringing - provisional response */