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 {