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):