mate: fix Match edge cases, improve documentation

Use strcmp to sort AVPs in an AVPL and for matching instead of comparing
pointer addresses. Pointers can only be used for (in)equality, there is
no ordering in them. Matching of attributes however requires a better
ordering to know whether the operator (condition) or whether the operand
(data) can be skipped. Otherwise it is possible that condition (b)
randomly fails to match data (a,b).

User-visible changes (mainly edge cases):

 - Loose (a=1, a?) on data (a=0, a=1) would previously fail to return
   (a=0,a=1) because the a? condition is not tried for data a=0. Now it
   tries all compatible conditions for a data AVP.
 - Any Match condition like (a=1, a^1) would previously be treated the
   same as (a=1) while (a^1, a=1) would still be seen as (a^1, a=1). The
   first case is now fixed to match (a=1, a^1). (Via a fix in insert_avp
   to ensure that (a=1) is not considered the same as (a^1).)
 - Every (a=1, a=2) on data (a=1, b=1) previously failed, but the
   comment "it will not create a list if there is not a match for every
   attribute in op" suggests that it should return (a=1).
 - Every (a=1) on data (a=2) previously succeeded (bug) while it would
   fail on (a=2, b=1). This is fixed now by checking whether any of the
   conditions really have matching data for the attribute.

Other changes: optimize merge_avpl and new_avpl_*_match to insert in
linear time instead of quadratic, rewrite and add comments in an attempt
to make it easier to understand. Merge the new_avpl_every_match and
new_avpl_exact_match functions and rename it to new_avpl_pairs_match to
reflects its actual implemented functionality.

Not addressed in this patch is the quasi-randomness of the returned
data AVPL. AVPLs are unordered, so the condition Strict (a?) on data
(a=1, a=2) could in theory return either (a=1) or (a=2). In practice
this returns (a=1) because of alphabetical ordering, but this cannot
really be relied on. It gets worse for conditions like Strict (a?, a>1),
these are considered undefined behavior (without warnings for now).

Ping-Bug: 12184
Change-Id: I0008448ffcb96183f106cb937c4f488e26a82f92
Reviewed-on: https://code.wireshark.org/review/17777
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 7 insertions, 7 deletions

diff --git a/plugins/mate/mate_runtime.c b/plugins/mate/mate_runtime.c
index d9455c99d1..1e4232c3c9 100644
--- a/plugins/mate/mate_runtime.c
+++ b/plugins/mate/mate_runtime.c
@@ -333,7 +333,7 @@ static void reanalyze_gop(mate_gop* gop) {
 		while (( curr_gogkey = get_next_avpl(gog_keys,&cookie) )) {
 			gop_cfg = (mate_cfg_gop *)g_hash_table_lookup(mc->gopcfgs,curr_gogkey->name);
 
-			if (( gogkey_match = new_avpl_exact_match(gop_cfg->name,gog->avpl,curr_gogkey,FALSE) )) {
+			if (( gogkey_match = new_avpl_pairs_match(gop_cfg->name, gog->avpl, curr_gogkey, TRUE, FALSE) )) {
 
 				gog_key = (gogkey *)g_malloc(sizeof(gogkey));
 
@@ -397,7 +397,7 @@ static void analyze_gop(mate_gop* gop) {
 		dbg_print (dbg_gog,1,dbg_facility,"analyze_gop: got gog_keys: %s",gog_keys->name) ;
 
 		while (( curr_gogkey = get_next_avpl(gog_keys,&cookie) )) {
-			if (( gogkey_match = new_avpl_exact_match(gop->cfg->name,gop->avpl,curr_gogkey,TRUE) )) {
+			if (( gogkey_match = new_avpl_pairs_match(gop->cfg->name, gop->avpl, curr_gogkey, TRUE, TRUE) )) {
 
 				key = avpl_to_str(gogkey_match);
 
@@ -484,7 +484,7 @@ static void analyze_pdu(mate_pdu* pdu) {
 	if (! (cfg = (mate_cfg_gop *)g_hash_table_lookup(mc->gops_by_pduname,pdu->cfg->name)) )
 		return;
 
-	if ((gopkey_match = new_avpl_exact_match("gop_key_match",pdu->avpl,cfg->key, TRUE))) {
+	if ((gopkey_match = new_avpl_pairs_match("gop_key_match", pdu->avpl, cfg->key, TRUE, TRUE))) {
 		gop_key = avpl_to_str(gopkey_match);
 
 		g_hash_table_lookup_extended(cfg->gop_index,(gconstpointer)gop_key,(gpointer *)&orig_gop_key,(gpointer *)&gop);
@@ -512,7 +512,7 @@ static void analyze_pdu(mate_pdu* pdu) {
 
 				dbg_print (dbg_gop,2,dbg_facility,"analyze_pdu: got candidate start");
 
-				if (( is_start = new_avpl_exact_match("",pdu->avpl, candidate_start, FALSE) )) {
+				if (( is_start = new_avpl_pairs_match("", pdu->avpl, candidate_start, TRUE, FALSE) )) {
 					delete_avpl(is_start,FALSE);
 					if ( gop->released ) {
 						dbg_print (dbg_gop,3,dbg_facility,"analyze_pdu: start on released gop, let's create a new gop");
@@ -551,7 +551,7 @@ static void analyze_pdu(mate_pdu* pdu) {
 				if (gog_keys) {
 
 					while (( curr_gogkey = get_next_avpl(gog_keys,&cookie) )) {
-						if (( gogkey_match = new_avpl_exact_match(cfg->name,gopkey_match,curr_gogkey,FALSE) )) {
+						if (( gogkey_match = new_avpl_pairs_match(cfg->name, gopkey_match, curr_gogkey, TRUE, FALSE) )) {
 							gogkey_str = avpl_to_str(gogkey_match);
 
 							if (g_hash_table_lookup(cfg->gog_index,gogkey_str)) {
@@ -582,7 +582,7 @@ static void analyze_pdu(mate_pdu* pdu) {
 			} else {
 				candidate_start = cfg->start;
 
-				if (( is_start = new_avpl_exact_match("",pdu->avpl, candidate_start, FALSE) )) {
+				if (( is_start = new_avpl_pairs_match("", pdu->avpl, candidate_start, TRUE, FALSE) )) {
 					delete_avpl(is_start,FALSE);
 					gop = new_gop(cfg,pdu,gop_key);
 				} else {
@@ -618,7 +618,7 @@ static void analyze_pdu(mate_pdu* pdu) {
 			candidate_stop = cfg->stop;
 
 			if (candidate_stop) {
-				is_stop = new_avpl_exact_match("",pdu->avpl, candidate_stop,FALSE);
+				is_stop = new_avpl_pairs_match("", pdu->avpl, candidate_stop, TRUE, FALSE);
 			} else {
 				is_stop = new_avpl("");
 			}