From 1400d9272441c0117e24100ed760780045732c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Valverde?= Date: Thu, 29 Dec 2022 20:06:19 +0000 Subject: [PATCH] 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 | 58 ++++++++++++++++-------------- epan/dfilter/dfilter-int.h | 5 +++ epan/dfilter/dfilter.c | 31 +++++++++++++--- epan/dfilter/dfilter.h | 4 +++ epan/dfilter/scanner.l | 26 +++++++------- epan/dfilter/semcheck.c | 21 +++++++++++ ui/qt/widgets/syntax_line_edit.cpp | 14 +++++--- 7 files changed, 110 insertions(+), 49 deletions(-) diff --git a/dftest.c b/dftest.c index 8fea7b121a..28b7e4bacb 100644 --- a/dftest.c +++ b/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); } /* diff --git a/epan/dfilter/dfilter-int.h b/epan/dfilter/dfilter-int.h index b477d7418a..de546d7cda 100644 --- a/epan/dfilter/dfilter-int.h +++ b/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); diff --git a/epan/dfilter/dfilter.c b/epan/dfilter/dfilter.c index e590d5152d..c5bd2dbe50 100644 --- a/epan/dfilter/dfilter.c +++ b/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) { diff --git a/epan/dfilter/dfilter.h b/epan/dfilter/dfilter.h index dd7282d6a1..4bfdc89335 100644 --- a/epan/dfilter/dfilter.h +++ b/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 diff --git a/epan/dfilter/scanner.l b/epan/dfilter/scanner.l index cefb9dc2d2..89005befbf 100644 --- a/epan/dfilter/scanner.l +++ b/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; } diff --git a/epan/dfilter/semcheck.c b/epan/dfilter/semcheck.c index 61499cc079..482be42178 100644 --- a/epan/dfilter/semcheck.c +++ b/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: diff --git a/ui/qt/widgets/syntax_line_edit.cpp b/ui/qt/widgets/syntax_line_edit.cpp index 376c028a29..69afe225f0 100644 --- a/ui/qt/widgets/syntax_line_edit.cpp +++ b/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(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);