dfilter: Add compilation warning for ambiguous syntax

$ dfilter 'frame contains fc'
    Filter: frame contains fc

    Warning: Interpreting "fc" as "Fibre Channel". Consider writing :fc or .fc.
    (...)

dftest.c

@@ -71,14 +71,15 @@ main(int argc, char **argv)
         cfile_write_failure_message,
         cfile_close_failure_message
     };
-    char		*text;
-    char		*expanded_text;
-    dfilter_t		*df;
-    gchar		*err_msg;
-    df_error_t          *df_err;
-    GTimer *timer;
+    char        *text = NULL;
+    char        *expanded_text = NULL;
+    dfilter_t   *df = NULL;
+    gchar       *err_msg = NULL;
+    df_error_t  *df_err = NULL;
+    GTimer      *timer = NULL;
     gdouble elapsed_expand, elapsed_compile;
     gboolean ok;
+    int exit_status = 0;
 
     cmdarg_err_init(dftest_cmdarg_err, dftest_cmdarg_err_cont);
 
@@ -162,13 +163,9 @@ main(int argc, char **argv)
     if (expanded_text == NULL) {
         fprintf(stderr, "dftest: %s\n", err_msg);
         g_free(err_msg);
-        g_free(text);
-        epan_cleanup();
-        g_timer_destroy(timer);
-        exit(2);
+        exit_status = 2;
+        goto out;
     }
-    g_free(text);
-    text = NULL;
 
     printf("Filter: %s\n", expanded_text);
 
@@ -186,28 +183,35 @@ main(int argc, char **argv)
             putloc(stderr, df_err->loc);
         }
         dfilter_error_free(df_err);
-        g_free(expanded_text);
-        epan_cleanup();
-        g_timer_destroy(timer);
-        exit(2);
+        exit_status = 2;
     }
 
     if (df == NULL) {
         printf("Filter is empty.\n");
-    }
-    else {
-        printf("\nSyntax tree:\n%s\n\n", dfilter_syntax_tree(df));
-        dfilter_dump(df);
-        printf("\nElapsed time: %.f µs (%.f µs + %.f µs)\n",
-                (elapsed_expand + elapsed_compile) * 1000 * 1000,
-                elapsed_expand * 1000 * 1000, elapsed_compile * 1000 * 1000);
+        goto out;
     }
 
-    dfilter_free(df);
+    for (GSList *l = dfilter_get_warnings(df); l != NULL; l = l->next) {
+        printf("\nWarning: %s.\n", (char *)l->data);
+    }
+
+    printf("\nSyntax tree:\n%s\n\n", dfilter_syntax_tree(df));
+    dfilter_dump(df);
+    printf("\nElapsed time: %.f µs (%.f µs + %.f µs)\n",
+            (elapsed_expand + elapsed_compile) * 1000 * 1000,
+            elapsed_expand * 1000 * 1000, elapsed_compile * 1000 * 1000);
+
+out:
     epan_cleanup();
-    g_free(expanded_text);
-    g_timer_destroy(timer);
-    exit(0);
+    if (df != NULL)
+        dfilter_free(df);
+    if (text != NULL)
+        g_free(text);
+    if (expanded_text != NULL)
+        g_free(expanded_text);
+    if (timer != NULL)
+        g_timer_destroy(timer);
+    exit(exit_status);
 }
 
 /*

epan/dfilter/dfilter-int.h

@@ -32,6 +32,7 @@ struct epan_dfilter {
 	int		*interesting_fields;
 	int		num_interesting_fields;
 	GPtrArray	*deprecated;
+	GSList		*warnings;
 	char		*expanded_text;
 	GHashTable	*references;
 	GHashTable	*raw_references;
@@ -60,6 +61,7 @@ typedef struct {
 	                                cleaning up memory allocations is inconvenient. Memory
 					allocated from this pool will be freed when the dfwork_t
 					context is destroyed. */
+	GSList		*warnings;
 } dfwork_t;
 
 /*
@@ -102,6 +104,9 @@ dfw_set_error_location(dfwork_t *dfw, df_loc_t err_loc);
 void
 add_deprecated_token(dfwork_t *dfw, const char *token);
 
+void
+add_compile_warning(dfwork_t *dfw, const char *format, ...);
+
 void
 free_deprecated(GPtrArray *deprecated);
 

epan/dfilter/dfilter.c

@@ -157,12 +157,10 @@ dfilter_new(GPtrArray *deprecated)
 
 	df = g_new0(dfilter_t, 1);
 	df->insns = NULL;
-
+	df->function_stack = NULL;
+	df->warnings = NULL;
 	if (deprecated)
 		df->deprecated = g_ptr_array_ref(deprecated);
-
-	df->function_stack = NULL;
-
 	return df;
 }
 
@@ -204,6 +202,9 @@ dfilter_free(dfilter_t *df)
 		g_slist_free(df->function_stack);
 	}
 
+	if (df->warnings)
+		g_slist_free_full(df->warnings, g_free);
+
 	g_free(df->registers);
 	g_free(df->attempted_load);
 	g_free(df->free_registers);
@@ -225,6 +226,7 @@ dfwork_new(void)
 	dfwork_t *dfw = g_new0(dfwork_t, 1);
 
 	dfw_error_init(&dfw->error);
+	dfw->warnings = NULL;
 
 	dfw->references =
 		g_hash_table_new_full(g_direct_hash, g_direct_equal,
@@ -273,6 +275,9 @@ dfwork_free(dfwork_t *dfw)
 	if (dfw->deprecated)
 		g_ptr_array_unref(dfw->deprecated);
 
+	if (dfw->warnings)
+		g_slist_free_full(dfw->warnings, g_free);
+
 	g_free(dfw->expanded_text);
 
 	wmem_destroy_allocator(dfw->dfw_scope);
@@ -347,6 +352,16 @@ add_deprecated_token(dfwork_t *dfw, const char *token)
 	g_ptr_array_add(deprecated, g_strdup(token));
 }
 
+void
+add_compile_warning(dfwork_t *dfw, const char *format, ...)
+{
+	va_list ap;
+	va_start(ap, format);
+	char *msg = ws_strdup_vprintf(format, ap);
+	va_end(ap);
+	dfw->warnings = g_slist_prepend(dfw->warnings, msg);
+}
+
 char *
 dfilter_expand(const char *expr, char **err_ret)
 {
@@ -515,6 +530,8 @@ dfilter_compile_real(const gchar *text, dfilter_t **dfp,
 		dfw->references = NULL;
 		dfilter->raw_references = dfw->raw_references;
 		dfw->raw_references = NULL;
+		dfilter->warnings = dfw->warnings;
+		dfw->warnings = NULL;
 
 		if (flags & DF_SAVE_TREE) {
 			ws_assert(tree_str);
@@ -601,6 +618,12 @@ dfilter_deprecated_tokens(dfilter_t *df) {
 	return NULL;
 }
 
+GSList *
+dfilter_get_warnings(dfilter_t *df)
+{
+	return df->warnings;
+}
+
 void
 dfilter_dump(dfilter_t *df)
 {

epan/dfilter/dfilter.h

@@ -117,6 +117,10 @@ WS_DLL_PUBLIC
 GPtrArray *
 dfilter_deprecated_tokens(dfilter_t *df);
 
+WS_DLL_PUBLIC
+GSList *
+dfilter_get_warnings(dfilter_t *df);
+
 /* Print bytecode of dfilter to stdout */
 WS_DLL_PUBLIC
 void

epan/dfilter/scanner.l

@@ -89,7 +89,7 @@ WS_WARN_UNUSED static int set_lval_literal(df_scanner_state_t *state, const char
 WS_WARN_UNUSED static int set_lval_unparsed(df_scanner_state_t *state, const char *token_value);
 WS_WARN_UNUSED static int set_lval_quoted_string(df_scanner_state_t *state, GString *quoted_string);
 WS_WARN_UNUSED static int set_lval_charconst(df_scanner_state_t *state, GString *quoted_string);
-WS_WARN_UNUSED static int set_lval_field(df_scanner_state_t *state, const char *token_value);
+WS_WARN_UNUSED static int set_lval_field(df_scanner_state_t *state, const char *token_value, const header_field_info *hfinfo);
 WS_WARN_UNUSED static int set_lval_identifier(df_scanner_state_t *state, const char *token_value);
 WS_WARN_UNUSED static int set_lval_constant(df_scanner_state_t *state, const char *token_value);
 
@@ -472,8 +472,13 @@ HyphenBytes	{hex2}(-{hex2})+
 \.{Identifier}			{
 	/* Field. */
 	update_location(yyextra, yytext);
-	/* Skip leading dot. */
-	return set_lval_field(yyextra, yytext + 1);
+	const char *name = yytext + 1;
+	header_field_info *hfinfo = dfilter_resolve_unparsed(yyextra->dfw, name);
+	if (hfinfo == NULL) {
+		FAIL("\"%s\" is not a valid protocol or protocol field.", name);
+		return SCAN_FAILED;
+	}
+	return set_lval_field(yyextra, yytext, hfinfo);
 }
 
 . {
@@ -546,10 +551,9 @@ set_lval_constant(df_scanner_state_t *state, const char *token_value)
 static int
 set_lval_unparsed(df_scanner_state_t *state, const char *token_value)
 {
-	header_field_info *hfinfo = dfilter_resolve_unparsed(state->dfw, token_value);
+	const header_field_info *hfinfo = dfilter_resolve_unparsed(state->dfw, token_value);
 	if (hfinfo != NULL) {
-		stnode_init(df_lval, STTYPE_FIELD, hfinfo, g_strdup(token_value), state->location);
-		return TOKEN_FIELD;
+		return set_lval_field(state, token_value, hfinfo);
     	}
 	return set_lval_literal(state, token_value);
 }
@@ -581,15 +585,9 @@ set_lval_charconst(df_scanner_state_t *state, GString *quoted_string)
 }
 
 static int
-set_lval_field(df_scanner_state_t *state, const char *token_value)
+set_lval_field(df_scanner_state_t *state, const char *token_value, const header_field_info *hfinfo)
 {
-	header_field_info *hfinfo;
-
-	hfinfo = dfilter_resolve_unparsed(state->dfw, token_value);
-	if (hfinfo == NULL) {
-		dfilter_fail(state->dfw, DF_ERROR_GENERIC, state->location, "\"%s\" is not a valid protocol or protocol field.", token_value);
-	}
-	stnode_init(df_lval, STTYPE_FIELD, hfinfo, g_strdup(token_value), state->location);
+	stnode_init(df_lval, STTYPE_FIELD, (gpointer)hfinfo, g_strdup(token_value), state->location);
 	return TOKEN_FIELD;
 }
 

epan/dfilter/semcheck.c

@@ -1032,12 +1032,33 @@ check_relation(dfwork_t *dfw, stnode_op_t st_op,
 	}
 }
 
+static void
+check_relation_contains_RHS_FIELD(dfwork_t *dfw, stnode_t *st_node _U_,
+		stnode_t *st_arg1 _U_, stnode_t *st_arg2)
+{
+	const char *token = stnode_token(st_arg2);
+	if (token[0] == '.' || token[0] == ':')
+		return;
+
+	header_field_info *hfinfo = sttype_field_hfinfo(st_arg2);
+	fvalue_t *fvalue = fvalue_from_literal(FT_BYTES, hfinfo->abbrev, FALSE, NULL);
+	if (fvalue != NULL) {
+		add_compile_warning(dfw, "Interpreting \"%s\" as \"%s\". Consider writing :%s or .%s",
+					hfinfo->abbrev, hfinfo->name, hfinfo->abbrev, hfinfo->abbrev);
+		fvalue_free(fvalue);
+	}
+}
+
 static void
 check_relation_contains(dfwork_t *dfw, stnode_t *st_node,
 		stnode_t *st_arg1, stnode_t *st_arg2)
 {
 	LOG_NODE(st_node);
 
+	if (stnode_type_id(st_arg2) == STTYPE_FIELD) {
+		check_relation_contains_RHS_FIELD(dfw, st_node, st_arg1, st_arg2);
+	}
+
 	switch (stnode_type_id(st_arg1)) {
 		case STTYPE_FIELD:
 		case STTYPE_REFERENCE:

ui/qt/widgets/syntax_line_edit.cpp

@@ -203,11 +203,17 @@ bool SyntaxLineEdit::checkDisplayFilter(QString filter)
     dfilter_t *dfp = NULL;
     df_error_t *df_err = NULL;
     if (dfilter_compile(filter.toUtf8().constData(), &dfp, &df_err)) {
+        GSList *warn;
         GPtrArray *depr = NULL;
-        if (dfp) {
-            depr = dfilter_deprecated_tokens(dfp);
-        }
-        if (depr) {
+        if (dfp != NULL && (warn = dfilter_get_warnings(dfp)) != NULL) {
+            // FIXME Need to use a different state or rename ::Deprecated
+            setSyntaxState(SyntaxLineEdit::Deprecated);
+            /*
+            * We're being lazy and only printing the first warning.
+            * Would it be better to print all of them?
+            */
+            syntax_error_message_  = QString(static_cast<gchar *>(warn->data));
+        } else if (dfp != NULL && (depr = dfilter_deprecated_tokens(dfp)) != NULL) {
             // You keep using that word. I do not think it means what you think it means.
             // Possible alternatives: ::Troubled, or ::Problematic maybe?
             setSyntaxState(SyntaxLineEdit::Deprecated);