Wireshark-dev: Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
From: Gerald Combs <gerald@xxxxxxxxxxxxx>
Date: Tue, 29 Jan 2008 16:07:47 -0800
Sake Blok wrote:
I think the idea of a pop-up explaining the way the operator "!=" works on fields with multiple occurences in one packet is a good way to educate people. But only if there is an option to "Don't show me this message again" :-)
The attached patch warns the user about "!=" and "ne" by coloring the filter entry yellow and adding a message to the status bar any time either of those tokens are present. It's a bit less obtrusive than popping up a dialog.
Index: gtk/main.c =================================================================== --- gtk/main.c (revision 24225) +++ gtk/main.c (working copy) @@ -225,7 +225,7 @@ static GtkWidget *packets_bar = NULL; static GtkWidget *profile_bar = NULL; static GtkWidget *welcome_pane; -static guint main_ctx, file_ctx, help_ctx; +static guint main_ctx, file_ctx, help_ctx, filter_ctx; static guint packets_ctx; static guint profile_ctx; static gchar *packets_str = NULL; @@ -1009,6 +1009,24 @@ } /* + * Push a message referring to the current filter onto the statusbar. + */ +void +statusbar_push_filter_msg(const gchar *msg) +{ + gtk_statusbar_push(GTK_STATUSBAR(info_bar), filter_ctx, msg); +} + +/* + * Pop a message referring to the current filter off the statusbar. + */ +void +statusbar_pop_filter_msg(void) +{ + gtk_statusbar_pop(GTK_STATUSBAR(info_bar), filter_ctx); +} + +/* * update the packets statusbar to the current values */ void packets_bar_update(void) @@ -3358,6 +3376,7 @@ main_ctx = gtk_statusbar_get_context_id(GTK_STATUSBAR(info_bar), "main"); file_ctx = gtk_statusbar_get_context_id(GTK_STATUSBAR(info_bar), "file"); help_ctx = gtk_statusbar_get_context_id(GTK_STATUSBAR(info_bar), "help"); + filter_ctx = gtk_statusbar_get_context_id(GTK_STATUSBAR(info_bar), "filter"); #if GTK_MAJOR_VERSION >= 2 gtk_statusbar_set_has_resize_grip(GTK_STATUSBAR(info_bar), FALSE); #endif Index: gtk/main.h =================================================================== --- gtk/main.h (revision 24225) +++ gtk/main.h (working copy) @@ -349,4 +349,16 @@ extern GtkWidget *pkt_scrollw; +/** Push a message referring to the current filter onto the statusbar. + * + * @param msg The message + */ +void +statusbar_push_filter_msg(const gchar *msg); + +/** Pop a message referring to the current filter off the statusbar. + */ +void +statusbar_pop_filter_msg(void); + #endif /* __MAIN_H__ */ Index: gtk/filter_dlg.c =================================================================== --- gtk/filter_dlg.c (revision 24225) +++ gtk/filter_dlg.c (working copy) @@ -1483,6 +1483,13 @@ } void +colorize_filter_te_as_deprecated(GtkWidget *w) +{ + /* light yellow */ + color_filter_te(w, 0xFFFF, 0xFFFF, 0xAFFF); +} + +void colorize_filter_te_as_valid(GtkWidget *w) { /* light green */ @@ -1494,19 +1501,39 @@ { const gchar *strval; dfilter_t *dfp; + GPtrArray *depr = NULL; + gchar *msg; + + statusbar_pop_filter_msg(); strval = gtk_entry_get_text(GTK_ENTRY(w)); /* colorize filter string entry */ if (strval && dfilter_compile(strval, &dfp)) { - if (dfp != NULL) + if (dfp != NULL) { + depr = dfilter_deprecated_tokens(dfp); dfilter_free(dfp); - if (strlen(strval) == 0) + } + if (strlen(strval) == 0) { colorize_filter_te_as_empty(w); - else + } else if (depr) { + /* You keep using that word. I do not think it means what you think it means. */ + colorize_filter_te_as_deprecated(w); + /* + * We're being lazy and only printing the first "problem" token. + * Would it be better to print all of them? + */ + msg = g_strdup_printf("\"%s\" may have unexpected results.", + (char *) g_ptr_array_index(depr, 0)); + statusbar_push_filter_msg(msg); + g_free(msg); + } else { colorize_filter_te_as_valid(w); - } else + } + } else { colorize_filter_te_as_invalid(w); + statusbar_push_filter_msg("Invalid filter"); + } } Index: epan/dfilter/scanner.l =================================================================== --- epan/dfilter/scanner.l (revision 24225) +++ epan/dfilter/scanner.l (working copy) @@ -83,6 +83,7 @@ static int simple(int token); static gboolean str_to_gint32(char *s, gint32* pint); GString* quoted_string = NULL; +static void mark_lval_deprecated(const char *s); %} @@ -102,8 +103,14 @@ "==" return simple(TOKEN_TEST_EQ); "eq" return simple(TOKEN_TEST_EQ); -"!=" return simple(TOKEN_TEST_NE); -"ne" return simple(TOKEN_TEST_NE); +"!=" { + mark_lval_deprecated("!="); + return simple(TOKEN_TEST_NE); +} +"ne" { + mark_lval_deprecated("ne"); + return simple(TOKEN_TEST_NE); +} ">" return simple(TOKEN_TEST_GT); "gt" return simple(TOKEN_TEST_GT); ">=" return simple(TOKEN_TEST_GE); @@ -417,4 +424,10 @@ return TRUE; } +static void +mark_lval_deprecated(const char *s) +{ + df_lval->deprecated_token = s; +} + #include <lemonflex-tail.inc> Index: epan/dfilter/syntax-tree.c =================================================================== --- epan/dfilter/syntax-tree.c (revision 24225) +++ epan/dfilter/syntax-tree.c (working copy) @@ -92,6 +92,7 @@ node = g_new(stnode_t, 1); node->magic = STNODE_MAGIC; + node->deprecated_token = NULL; if (type_id == STTYPE_UNINITIALIZED) { node->type = NULL; @@ -188,3 +189,12 @@ assert_magic(node, STNODE_MAGIC); return node->value; } + +char * +stnode_deprecated(stnode_t *node) +{ + if (!node) { + return NULL; + } + return node->deprecated_token; +} Index: epan/dfilter/syntax-tree.h =================================================================== --- epan/dfilter/syntax-tree.h (revision 24225) +++ epan/dfilter/syntax-tree.h (working copy) @@ -60,6 +60,7 @@ * set aside to time to do so. */ gpointer data; gint32 value; + char *deprecated_token; } stnode_t; /* These are the sttype_t registration function prototypes. */ @@ -103,6 +104,9 @@ gint32 stnode_value(stnode_t *node); +char * +stnode_deprecated(stnode_t *node); + #define assert_magic(obj, mnum) \ g_assert((obj)); \ if ((obj)->magic != (mnum)) { \ Index: epan/dfilter/dfilter.c =================================================================== --- epan/dfilter/dfilter.c (revision 24225) +++ epan/dfilter/dfilter.c (working copy) @@ -115,6 +115,7 @@ df = g_new(dfilter_t, 1); df->insns = NULL; + df->deprecated = NULL; return df; } @@ -219,6 +220,9 @@ dfilter_t *dfilter; dfwork_t *dfw; gboolean failure = FALSE; + char *depr_test; + guint i; + GPtrArray *deprecated = g_ptr_array_new(); dfilter_error_msg = NULL; @@ -245,6 +249,22 @@ break; } + /* See if the node is deprecated */ + depr_test = stnode_deprecated(df_lval); + + if (depr_test) { + for (i = 0; i < deprecated->len; i++) { + if (strcasecmp(depr_test, g_ptr_array_index(deprecated, i)) == 0) { + /* It's already in our list */ + depr_test = NULL; + } + } + } + + if (depr_test) { + g_ptr_array_add(deprecated, depr_test); + } + /* Give the token to the parser */ Dfilter(ParserObj, token, df_lval, dfw); /* We've used the stnode_t, so we don't want to free it */ @@ -254,6 +274,7 @@ failure = TRUE; break; } + } /* while (1) */ /* If we created an stnode_t but didn't use it, free it; the @@ -285,6 +306,7 @@ * it and set *dfp to NULL */ if (dfw->st_root == NULL) { *dfp = NULL; + g_ptr_array_free(deprecated, TRUE); } else { @@ -314,6 +336,9 @@ /* Initialize constants */ dfvm_init_const(dfilter); + /* Add any deprecated items */ + dfilter->deprecated = deprecated; + /* And give it to the user. */ *dfp = dfilter; } @@ -325,6 +350,7 @@ if (dfw) { dfwork_free(dfw); } + g_ptr_array_free(deprecated, TRUE); dfilter_fail("Unable to parse filter string \"%s\".", text); *dfp = NULL; return FALSE; @@ -355,9 +381,28 @@ } } +GPtrArray * +dfilter_deprecated_tokens(dfilter_t *df) { + if (df->deprecated && df->deprecated->len > 0) { + return df->deprecated; + } + return NULL; +} void dfilter_dump(dfilter_t *df) { + guint i; + gchar *sep = ""; + dfvm_dump(stdout, df->insns); + + if (df->deprecated && df->deprecated->len) { + printf("\nDeprecated tokens: "); + for (i = 0; i < df->deprecated->len; i++) { + printf("%s\"%s\"", sep, (char *) g_ptr_array_index(df->deprecated, i)); + sep = ", "; + } + printf("\n"); + } } Index: epan/dfilter/dfilter.h =================================================================== --- epan/dfilter/dfilter.h (revision 24225) +++ epan/dfilter/dfilter.h (working copy) @@ -84,6 +84,9 @@ void dfilter_prime_proto_tree(const dfilter_t *df, proto_tree *tree); +GPtrArray * +dfilter_deprecated_tokens(dfilter_t *df); + /* Print bytecode of dfilter to stdout */ void dfilter_dump(dfilter_t *df); Index: epan/dfilter/gencode.c =================================================================== Index: epan/dfilter/dfilter-int.h =================================================================== --- epan/dfilter/dfilter-int.h (revision 24225) +++ epan/dfilter/dfilter-int.h (working copy) @@ -39,6 +39,7 @@ gboolean *attempted_load; int *interesting_fields; int num_interesting_fields; + GPtrArray *deprecated; }; typedef struct {
- Follow-Ups:
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: Stig Bjørlykke
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- References:
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: John McDermott
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: Kenichi Okuyama
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: Sake Blok
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: Stig Bjørlykke
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: Jaap Keuter
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- From: Sake Blok
- Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- Prev by Date: Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- Next by Date: Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- Previous by thread: Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- Next by thread: Re: [Wireshark-dev] ip.addr != 10.0.0.1 (Guy Harris)
- Index(es):