[Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>]

On Fri, Aug 09, 2013 at 09:46:29PM +0200, Jakub Zawadzki wrote:
> On Fri, Aug 09, 2013 at 12:26:53PM -0700, Guy Harris wrote:
> > > but +1 for being consistent with the rest of the world.
> > 
> > Yes.
> 
> I'd rather not change grammar, still I think what we need - is some warning:
>   "suggest parentheses around '&&' within '||'"

Can anyone look at attached patch? It seems to be working OK...

diff --git a/epan/dfilter/dfilter.c b/epan/dfilter/dfilter.c
index bef5e7e..dc669e1 100644
--- a/epan/dfilter/dfilter.c
+++ b/epan/dfilter/dfilter.c
@@ -318,7 +318,7 @@ dfilter_compile(const gchar *text, dfilter_t **dfp)
 	else {
 
 		/* Check semantics and do necessary type conversion*/
-		if (!dfw_semcheck(dfw)) {
+		if (!dfw_semcheck(dfw, deprecated)) {
 			goto FAILURE;
 		}
 
diff --git a/epan/dfilter/grammar.lemon b/epan/dfilter/grammar.lemon
index 88c59b6..a808d1b 100644
--- a/epan/dfilter/grammar.lemon
+++ b/epan/dfilter/grammar.lemon
@@ -309,5 +309,6 @@ funcparams(P) ::= funcparams(L) COMMA entity(E).
 expr(X) ::= LPAREN expr(Y) RPAREN.
 {
 	X = Y;
+	stnode_set_bracket(X, TRUE);
 }
 
diff --git a/epan/dfilter/semcheck.c b/epan/dfilter/semcheck.c
index 47dd748..0e572f1 100644
--- a/epan/dfilter/semcheck.c
+++ b/epan/dfilter/semcheck.c
@@ -49,7 +49,7 @@
 #endif
 
 static void
-semcheck(stnode_t *st_node);
+semcheck(stnode_t *st_node, GPtrArray *deprecated);
 
 static stnode_t*
 check_param_entity(stnode_t *st_node);
@@ -1214,7 +1214,7 @@ header_field_info   *hfinfo;
 
 /* Check the semantics of any type of TEST */
 static void
-check_test(stnode_t *st_node)
+check_test(stnode_t *st_node, GPtrArray *deprecated)
 {
 	test_op_t		st_op;
 	stnode_t		*st_arg1, *st_arg2;
@@ -1236,13 +1236,38 @@ check_test(stnode_t *st_node)
 			break;
 
 		case TEST_OP_NOT:
-			semcheck(st_arg1);
+			semcheck(st_arg1, deprecated);
 			break;
 
 		case TEST_OP_AND:
 		case TEST_OP_OR:
-			semcheck(st_arg1);
-			semcheck(st_arg2);
+			while (stnode_type_id(st_arg1) == STTYPE_TEST && stnode_type_id(st_arg2) == STTYPE_TEST) {
+				test_op_t st_arg1_op, st_arg2_op;
+
+				/* Warn user */
+				/* st_node->deprecated_token = "suggest parentheses around '&&' within '||'";; */
+				/* XXX, don't duplicate this message, iterate over and don't add if already exists */
+
+				sttype_test_get(st_arg1, &st_arg1_op, NULL, NULL);
+				if (st_arg1_op == TEST_OP_AND || st_arg1_op == TEST_OP_OR) {
+					if (st_op != st_arg1_op && !st_arg1->inside_brackets) {
+						g_ptr_array_add(deprecated, g_strdup("suggest parentheses around '&&' within '||'"));
+						break;
+					}
+				}
+
+				sttype_test_get(st_arg2, &st_arg2_op, NULL, NULL);
+				if (st_arg2_op == TEST_OP_AND || st_arg2_op == TEST_OP_OR) {
+					if (st_op != st_arg2_op && !st_arg2->inside_brackets) {
+						g_ptr_array_add(deprecated, g_strdup("suggest parentheses around '&&' within '||'"));
+						break;
+					}
+				}
+
+				break;
+			}
+			semcheck(st_arg1, deprecated);
+			semcheck(st_arg2, deprecated);
 			break;
 
 		case TEST_OP_EQ:
@@ -1281,7 +1306,7 @@ check_test(stnode_t *st_node)
 
 /* Check the entire syntax tree. */
 static void
-semcheck(stnode_t *st_node)
+semcheck(stnode_t *st_node, GPtrArray *deprecated)
 {
 #ifdef DEBUG_dfilter
 	static guint i = 0;
@@ -1291,7 +1316,7 @@ semcheck(stnode_t *st_node)
 	 * node will be a TEST node, no matter what. So assert that. */
 	switch (stnode_type_id(st_node)) {
 		case STTYPE_TEST:
-			check_test(st_node);
+			check_test(st_node, deprecated);
 			break;
 		default:
 			g_assert_not_reached();
@@ -1303,7 +1328,7 @@ semcheck(stnode_t *st_node)
  * some of the nodes into the form they need to be in order to
  * later generate the DFVM bytecode. */
 gboolean
-dfw_semcheck(dfwork_t *dfw)
+dfw_semcheck(dfwork_t *dfw, GPtrArray *deprecated)
 {
 	volatile gboolean ok_filter = TRUE;
 #ifdef DEBUG_dfilter
@@ -1315,7 +1340,7 @@ dfw_semcheck(dfwork_t *dfw)
 	 * the semantic-checking, the semantic-checking code will
 	 * throw an exception if a problem is found. */
 	TRY {
-		semcheck(dfw->st_root);
+		semcheck(dfw->st_root, deprecated);
 	}
 	CATCH(TypeError) {
 		ok_filter = FALSE;
diff --git a/epan/dfilter/semcheck.h b/epan/dfilter/semcheck.h
index e3076ae..36cfe3f 100644
--- a/epan/dfilter/semcheck.h
+++ b/epan/dfilter/semcheck.h
@@ -25,7 +25,7 @@
 #define SEMCHECK_H
 
 gboolean
-dfw_semcheck(dfwork_t *dfw);
+dfw_semcheck(dfwork_t *dfw, GPtrArray *deprecated);
 
 
 #endif
diff --git a/epan/dfilter/syntax-tree.c b/epan/dfilter/syntax-tree.c
index 1431814..abede8f 100644
--- a/epan/dfilter/syntax-tree.c
+++ b/epan/dfilter/syntax-tree.c
@@ -91,6 +91,7 @@ stnode_new(sttype_id_t type_id, gpointer data)
 	node = g_new(stnode_t, 1);
 	node->magic = STNODE_MAGIC;
         node->deprecated_token = NULL;
+	node->inside_brackets = FALSE;
 
 	if (type_id == STTYPE_UNINITIALIZED) {
 		node->type = NULL;
@@ -112,6 +113,12 @@ stnode_new(sttype_id_t type_id, gpointer data)
 	return node;
 }
 
+void
+stnode_set_bracket(stnode_t *node, gboolean bracket)
+{
+	node->inside_brackets = bracket;
+}
+
 stnode_t*
 stnode_dup(const stnode_t *org)
 {
@@ -132,6 +139,7 @@ stnode_dup(const stnode_t *org)
 	else
 		node->data = org->data;
 	node->value = org->value;
+	node->inside_brackets = org->inside_brackets;
 
 	return node;
 }
diff --git a/epan/dfilter/syntax-tree.h b/epan/dfilter/syntax-tree.h
index 6bf80fc..2a53538 100644
--- a/epan/dfilter/syntax-tree.h
+++ b/epan/dfilter/syntax-tree.h
@@ -65,6 +65,7 @@ typedef struct {
 	 * set aside to time to do so. */
 	gpointer	data;
 	gint32		value;
+	gboolean        inside_brackets;
         const char      *deprecated_token;
 } stnode_t;
 
@@ -88,6 +89,9 @@ sttype_register(sttype_t *type);
 stnode_t*
 stnode_new(sttype_id_t type_id, gpointer data);
 
+void
+stnode_set_bracket(stnode_t *node, gboolean bracket);
+
 stnode_t*
 stnode_dup(const stnode_t *org);