@rfuchs requested changes on this pull request.

Couple of suggestions on how to improve the flow of logic here. You can leave them unchanged if you prefer, then I will address them myself afterwards.

However, please do document these changes in doc/rtpengine_admin.xml, perhaps with an explanation of how they might be useful.


In src/modules/rtpengine/rtpengine.c:

> @@ -1784,6 +1784,8 @@ static int parse_flags(struct ng_flags_parse *ng_flags, struct sip_msg *msg, enu
 			case 6:
 				if (str_eq(&key, "to-tag")) {
 					ng_flags->to = 1;
+					if (val.s && val.len > 0)

If a value is given, you could simply leave ng_flags->to alone (as zero) and goto generic


In src/modules/rtpengine/rtpengine.c:

> @@ -1967,8 +1970,13 @@ static bencode_item_t *rtpp_function_call(bencode_buffer_t *bencbuf, struct sip_
 	if (ng_flags.rtcp_mux && ng_flags.rtcp_mux->child)
 		bencode_dictionary_add(ng_flags.dict, "rtcp-mux", ng_flags.rtcp_mux);
 
-	bencode_dictionary_add_str(ng_flags.dict, "call-id", &callid);
-
+        temp.s = NULL;
+        if (!bencode_dictionary_get_str(ng_flags.dict, "call-id", &temp))

You can get rid of the temp variable and the case distinction below by simply retrieving the given call ID into callid. Otherwise, at the very least please rename temp to something more meaningful, as there's quite a long distance between this and the place where it's actually being used.

Generally though, instead of doing a _get_str here, I would prefer parse_flags to take care of this and update a flag in ng_flags to signal that a call ID is already present. (Reason is that a dictionary lookup for a constructed dictionary reverts to a linear search, and I cringe at linear searches. 😄 ) The same applies to the fromtag/totag handling below, which can be solved in a similar way.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.