Re: [sr-dev] [kamailio/kamailio] rtpengine: Delete flags & delete handling improvement (#1103)

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. 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` 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. :smile: ) 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 or view it on GitHub: https://github.com/kamailio/kamailio/pull/1103#pullrequestreview-35371108