dfilter: Use syntax tree node replacement semantics

Instead of using 3 operations (new + free + reassign_to_parent) to transform
the tree use a simpler single replace operation instead.

This also avoids having to manually copy token values.

The set search and replace method is now obsolete.

5 files changed, 72 insertions, 88 deletions

diff --git a/epan/dfilter/semcheck.c b/epan/dfilter/semcheck.c
index 38d79dfc3a..e58561b01e 100644
--- a/epan/dfilter/semcheck.c
+++ b/epan/dfilter/semcheck.c
@@ -756,7 +756,7 @@ check_relation_LHS_FIELD(dfwork_t *dfw, const char *relation_string,
 			if (!pcre) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_PCRE, pcre, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_PCRE, pcre);
 		} else {
 			/* Skip incompatible fields */
 			while (hfinfo1->same_name_prev_id != -1 &&
@@ -798,15 +798,8 @@ check_relation_LHS_FIELD(dfwork_t *dfw, const char *relation_string,
 			if (!fvalue) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 		}
-		if (stnode_type_id(st_node) == STTYPE_TEST) {
-			sttype_test_set2_args(st_node, st_arg1, new_st);
-		} else {
-			/* Replace STTYPE_UNPARSED element by resolved value. */
-			sttype_set_replace_element(st_node, st_arg2, new_st);
-		}
-		stnode_free(st_arg2);
 	}
 	else if (type2 == STTYPE_RANGE) {
 		check_drange_sanity(dfw, st_arg2);
@@ -893,10 +886,9 @@ check_relation_LHS_FIELD(dfwork_t *dfw, const char *relation_string,
 static void
 check_relation_LHS_STRING(dfwork_t *dfw, const char* relation_string,
 		FtypeCanFunc can_func, gboolean allow_partial_value _U_,
-		stnode_t *st_node,
+		stnode_t *st_node _U_,
 		stnode_t *st_arg1, stnode_t *st_arg2)
 {
-	stnode_t		*new_st;
 	sttype_id_t		type2;
 	header_field_info	*hfinfo2;
 	df_func_def_t		*funcdef;
@@ -929,9 +921,7 @@ check_relation_LHS_STRING(dfwork_t *dfw, const char* relation_string,
 			}
 		}
 
-		new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg1->token_value);
-		sttype_test_set2_args(st_node, new_st, st_arg2);
-		stnode_free(st_arg1);
+		stnode_replace(st_arg1, STTYPE_FVALUE, fvalue);
 	}
 	else if (type2 == STTYPE_STRING || type2 == STTYPE_UNPARSED ||
 	         type2 == STTYPE_CHARCONST) {
@@ -948,9 +938,7 @@ check_relation_LHS_STRING(dfwork_t *dfw, const char* relation_string,
 		if (!fvalue) {
 			THROW(TypeError);
 		}
-		new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg1->token_value);
-		sttype_test_set2_args(st_node, new_st, st_arg2);
-		stnode_free(st_arg1);
+		stnode_replace(st_arg1, STTYPE_FVALUE, fvalue);
 	}
 	else if (type2 == STTYPE_FUNCTION) {
 		funcdef = sttype_function_funcdef(st_arg2);
@@ -971,9 +959,7 @@ check_relation_LHS_STRING(dfwork_t *dfw, const char* relation_string,
 
 		check_function(dfw, st_arg2);
 
-		new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg1->token_value);
-		sttype_test_set2_args(st_node, new_st, st_arg2);
-		stnode_free(st_arg1);
+		stnode_replace(st_arg1, STTYPE_FVALUE, fvalue);
 	}
 	else if (type2 == STTYPE_SET) {
 		dfilter_fail(dfw, "Only a field may be tested for membership in a set.");
@@ -987,10 +973,9 @@ check_relation_LHS_STRING(dfwork_t *dfw, const char* relation_string,
 static void
 check_relation_LHS_UNPARSED(dfwork_t *dfw, const char* relation_string,
 		FtypeCanFunc can_func, gboolean allow_partial_value,
-		stnode_t *st_node,
+		stnode_t *st_node _U_,
 		stnode_t *st_arg1, stnode_t *st_arg2)
 {
-	stnode_t		*new_st;
 	sttype_id_t		type2;
 	header_field_info	*hfinfo2;
 	df_func_def_t		*funcdef;
@@ -1023,9 +1008,7 @@ check_relation_LHS_UNPARSED(dfwork_t *dfw, const char* relation_string,
 			}
 		}
 
-		new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg1->token_value);
-		sttype_test_set2_args(st_node, new_st, st_arg2);
-		stnode_free(st_arg1);
+		stnode_replace(st_arg1, STTYPE_FVALUE, fvalue);
 	}
 	else if (type2 == STTYPE_STRING || type2 == STTYPE_UNPARSED ||
 	         type2 == STTYPE_CHARCONST) {
@@ -1042,9 +1025,7 @@ check_relation_LHS_UNPARSED(dfwork_t *dfw, const char* relation_string,
 		if (!fvalue) {
 			THROW(TypeError);
 		}
-		new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg1->token_value);
-		sttype_test_set2_args(st_node, new_st, st_arg2);
-		stnode_free(st_arg1);
+		stnode_replace(st_arg1, STTYPE_FVALUE, fvalue);
 	}
 	else if (type2 == STTYPE_FUNCTION) {
 		funcdef = sttype_function_funcdef(st_arg2);
@@ -1065,9 +1046,7 @@ check_relation_LHS_UNPARSED(dfwork_t *dfw, const char* relation_string,
 
 		check_function(dfw, st_arg2);
 
-		new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg1->token_value);
-		sttype_test_set2_args(st_node, new_st, st_arg2);
-		stnode_free(st_arg1);
+		stnode_replace(st_arg1, STTYPE_FVALUE, fvalue);
 	}
 	else if (type2 == STTYPE_SET) {
 		dfilter_fail(dfw, "Only a field may be tested for membership in a set.");
@@ -1128,16 +1107,14 @@ check_relation_LHS_RANGE(dfwork_t *dfw, const char *relation_string,
 			if (!pcre) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_PCRE, pcre, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_PCRE, pcre);
 		} else {
 			fvalue = dfilter_fvalue_from_string(dfw, FT_BYTES, s);
 			if (!fvalue) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 		}
-		sttype_test_set2_args(st_node, st_arg1, new_st);
-		stnode_free(st_arg2);
 	}
 	else if (type2 == STTYPE_UNPARSED) {
 		ws_debug("5 check_relation_LHS_RANGE(type2 = STTYPE_UNPARSED)");
@@ -1149,7 +1126,7 @@ check_relation_LHS_RANGE(dfwork_t *dfw, const char *relation_string,
 			if (!pcre) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_PCRE, pcre, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_PCRE, pcre);
 		} else {
 			/*
 			 * The RHS should be FT_BYTES. However, there is a
@@ -1186,10 +1163,8 @@ check_relation_LHS_RANGE(dfwork_t *dfw, const char *relation_string,
 			if (!fvalue) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 		}
-		sttype_test_set2_args(st_node, st_arg1, new_st);
-		stnode_free(st_arg2);
 	}
 	else if (type2 == STTYPE_CHARCONST) {
 		ws_debug("5 check_relation_LHS_RANGE(type2 = STTYPE_CHARCONST)");
@@ -1200,7 +1175,7 @@ check_relation_LHS_RANGE(dfwork_t *dfw, const char *relation_string,
 			if (!pcre) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_PCRE, pcre, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_PCRE, pcre);
 		} else {
 			/* The RHS should be FT_BYTES, but a character is just a
 			 * one-byte byte string. */
@@ -1208,10 +1183,8 @@ check_relation_LHS_RANGE(dfwork_t *dfw, const char *relation_string,
 			if (!fvalue) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 		}
-		sttype_test_set2_args(st_node, st_arg1, new_st);
-		stnode_free(st_arg2);
 	}
 	else if (type2 == STTYPE_RANGE) {
 		ws_debug("5 check_relation_LHS_RANGE(type2 = STTYPE_RANGE)");
@@ -1332,16 +1305,14 @@ check_relation_LHS_FUNCTION(dfwork_t *dfw, const char *relation_string,
 			if (!pcre) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_PCRE, pcre, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_PCRE, pcre);
 		} else {
 			fvalue = dfilter_fvalue_from_string(dfw, ftype1, s);
 			if (!fvalue) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 		}
-		sttype_test_set2_args(st_node, st_arg1, new_st);
-		stnode_free(st_arg2);
 	}
 	else if (type2 == STTYPE_UNPARSED || type2 == STTYPE_CHARCONST) {
 		s = (char*)stnode_data(st_arg2);
@@ -1351,16 +1322,14 @@ check_relation_LHS_FUNCTION(dfwork_t *dfw, const char *relation_string,
 			if (!pcre) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_PCRE, pcre, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_PCRE, pcre);
 		} else {
 			fvalue = dfilter_fvalue_from_unparsed(dfw, ftype1, s, allow_partial_value);
 			if (!fvalue) {
 				THROW(TypeError);
 			}
-			new_st = stnode_new(STTYPE_FVALUE, fvalue, st_arg2->token_value);
+			stnode_replace(st_arg2, STTYPE_FVALUE, fvalue);
 		}
-		sttype_test_set2_args(st_node, st_arg1, new_st);
-		stnode_free(st_arg2);
 	}
 	else if (type2 == STTYPE_RANGE) {
 		check_drange_sanity(dfw, st_arg2);
@@ -1419,7 +1388,6 @@ check_relation(dfwork_t *dfw, const char *relation_string,
 	static guint i = 0;
 #endif
 	header_field_info   *hfinfo;
-	stnode_t            *new_st;
 	char                *s;
 
 	ws_debug("4 check_relation(\"%s\") [%u]", relation_string, i++);
@@ -1460,10 +1428,7 @@ check_relation(dfwork_t *dfw, const char *relation_string,
 			 * functions will take care of it as if it didn't
 			 * match a protocol string.
 			 */
-			new_st = stnode_new(STTYPE_UNPARSED, s, st_arg2->token_value);
-			stnode_free(st_arg2);
-			st_arg2 = new_st;
-			sttype_test_set2_args(st_node, st_arg1, new_st);
+			stnode_replace(st_arg2, STTYPE_UNPARSED, s);
 		}
 	}
 
diff --git a/epan/dfilter/sttype-set.c b/epan/dfilter/sttype-set.c
index 829003bc15..52dcaefaf7 100644
--- a/epan/dfilter/sttype-set.c
+++ b/epan/dfilter/sttype-set.c
@@ -76,23 +76,6 @@ sttype_set_tostr(const void *data)
 }
 
 void
-sttype_set_replace_element(stnode_t *node, stnode_t *oldnode, stnode_t *newnode)
-{
-	GSList	*nodelist = (GSList*)stnode_data(node);
-
-	/* This deliberately checks both the left and right nodes, covering both
-	 * the lower and upper bound for ranges. NULL right nodes (in case of
-	 * normal, non-range elements) will usually not match "oldnode". */
-	while (nodelist) {
-		if (nodelist->data == oldnode) {
-			nodelist->data = newnode;
-			break;
-		}
-		nodelist = g_slist_next(nodelist);
-	}
-}
-
-void
 sttype_register_set(void)
 {
 	static sttype_t set_type = {
diff --git a/epan/dfilter/sttype-set.h b/epan/dfilter/sttype-set.h
index 8faad34b8f..53ac6b463f 100644
--- a/epan/dfilter/sttype-set.h
+++ b/epan/dfilter/sttype-set.h
@@ -14,9 +14,6 @@
 
 #include "ws_attributes.h"
 
-void
-sttype_set_replace_element(stnode_t *node, stnode_t *oldnode, stnode_t *newnode);
-
 gboolean
 sttype_set_convert_to_range(stnode_t **node_left, stnode_t **node_right);
 
diff --git a/epan/dfilter/syntax-tree.c b/epan/dfilter/syntax-tree.c
index 5a58e54080..6741a7c537 100644
--- a/epan/dfilter/syntax-tree.c
+++ b/epan/dfilter/syntax-tree.c
@@ -76,15 +76,41 @@ sttype_lookup(sttype_id_t type_id)
 	return result;
 }
 
+static void
+_node_clear(stnode_t *node)
+{
+	ws_assert_magic(node, STNODE_MAGIC);
+	if (node->type) {
+		if (node->type->func_free && node->data) {
+			node->type->func_free(node->data);
+		}
+	}
+	else {
+		ws_assert(!node->data);
+	}
+
+	node->type = NULL;
+	node->flags = 0;
+	node->data = NULL;
+	node->value = 0;
+}
+
 void
-stnode_init(stnode_t *node, sttype_id_t type_id, gpointer data,  const char *token_value)
+stnode_clear(stnode_t *node)
+{
+	_node_clear(node);
+	g_free(node->token_value);
+	node->token_value = NULL;
+}
+
+static void
+_node_init(stnode_t *node, sttype_id_t type_id, gpointer data)
 {
 	sttype_t	*type;
 
 	ws_assert_magic(node, STNODE_MAGIC);
 	ws_assert(!node->type);
 	ws_assert(!node->data);
-	ws_assert(!node->token_value);
 	node->flags = 0;
 	node->value = 0;
 
@@ -102,8 +128,14 @@ stnode_init(stnode_t *node, sttype_id_t type_id, gpointer data,  const char *tok
 		else {
 			node->data = data;
 		}
-
 	}
+}
+
+void
+stnode_init(stnode_t *node, sttype_id_t type_id, gpointer data,  const char *token_value)
+{
+	_node_init(node, type_id, data);
+	ws_assert(node->token_value == NULL);
 	node->token_value = g_strdup(token_value);
 }
 
@@ -114,6 +146,15 @@ stnode_init_int(stnode_t *node, sttype_id_t type_id, gint32 value, const char *t
 	node->value = value;
 }
 
+void
+stnode_replace(stnode_t *node, sttype_id_t type_id, gpointer data)
+{
+	uint16_t flags = node->flags; /* Save flags. */
+	_node_clear(node);
+	_node_init(node, type_id, data);
+	node->flags = flags;
+}
+
 stnode_t*
 stnode_new(sttype_id_t type_id, gpointer data, const char *token_value)
 {
@@ -159,15 +200,7 @@ void
 stnode_free(stnode_t *node)
 {
 	ws_assert_magic(node, STNODE_MAGIC);
-	if (node->type) {
-		if (node->type->func_free) {
-			node->type->func_free(node->data);
-		}
-	}
-	else {
-		ws_assert(!node->data);
-	}
-	g_free(node->token_value);
+	stnode_clear(node);
 	g_free(node);
 }
 
diff --git a/epan/dfilter/syntax-tree.h b/epan/dfilter/syntax-tree.h
index 7a7f94d625..4494dcbddd 100644
--- a/epan/dfilter/syntax-tree.h
+++ b/epan/dfilter/syntax-tree.h
@@ -91,12 +91,18 @@ stnode_t*
 stnode_dup(const stnode_t *org);
 
 void
+stnode_clear(stnode_t *node);
+
+void
 stnode_init(stnode_t *node, sttype_id_t type_id, gpointer data, const char *token_value);
 
 void
 stnode_init_int(stnode_t *node, sttype_id_t type_id, gint32 value, const char *token_value);
 
 void
+stnode_replace(stnode_t *node, sttype_id_t type_id, gpointer data);
+
+void
 stnode_free(stnode_t *node);
 
 const char*