Wireshark · Ethereal-dev: Re: [Ethereal-dev] [Patch] to packet-diameter.c

[Martin Mathieson <martin.mathieson@xxxxxxxxxxxx>]

Joerg Mayer wrote:

> On Tue, Mar 14, 2006 at 02:03:03PM +0000, Martin Mathieson wrote:
>  
>
> > This patch:
> > - makes it possible to turn off use of the XML AVP dictionary (which 
> > relies upon the XML lib being installed).  A failed load results in 3 
> > annoying dialogs popping up the first time a diameter packet is read.  
> > Default is previous behaviour.
> >    
>
> Maybe you can add a fix so that only one requester pop up in the default
> case?
>
> thanks
>      Joerg
>
>  
This updated patch improves things a little by removing one of the 
popups which was redundant, i.e. if xmlParseFilePush() fails, it prints 
out a specific error that will include the name of the file it tried to 
load (by default diameter/dictionary.xml), so there is no need for the 
general failure message that followed.

The last popup tells you that the default dictionary (from 
packet-diameter-defs.h) will be used instead, which seems fair enough.

As I've noted in a comment, InitializeDictionary() still only ever gets 
called once (in dissect_diameter_common()), so the dictionary change 
will only be seen once Ethereal has been restarted (the AVP lists are 
never freed).  I'll fix that too if it annoys me enough :)

Regards,
Martin


Index: epan/dissectors/packet-diameter.c
===================================================================
--- epan/dissectors/packet-diameter.c	(revision 17626)
+++ epan/dissectors/packet-diameter.c	(working copy)
@@ -246,6 +246,7 @@
 /* Suppress console output at unknown AVP:s,Flags etc */
 static gboolean suppress_console_output = TRUE;
 
+static gboolean gbl_use_xml_dictionary = TRUE;
 #define DICT_FN  "diameter/dictionary.xml"
 static const gchar *gbl_diameterDictionary;
 
@@ -665,7 +666,7 @@
   ApplicationId *entry;
 
   if (!name || (id == 0 && !allow_zero_as_app_id)) {
-	report_failure( "Diameter Error: Invalid application (name=%p, id=%d)",
+	report_failure( "Diameter Error: Invalid application (name=%s, id=%d)",
 			   name, id);
 	return (-1);
   } /* Sanity Checks */
@@ -836,10 +837,12 @@
   XmlStub.xmlSubstituteEntitiesDefault(1);            /* Substitute entities automagically */
   doc = xmlParseFilePush(gbl_diameterDictionary, 1);  /* Parse the XML (do validity checks)*/
 
-  /* Check for invalid xml */
+  /* Check for invalid xml.
+     Note that xmlParseFilePush reports details of problems found,
+     and it should be obvious from the default filename that the error relates
+     to Diameter.
+  */
   if (doc == NULL) {
-	report_failure("Diameter: Unable to parse xmldictionary %s",
-			  gbl_diameterDictionary);
 	return -1;
   }
 
@@ -918,9 +921,9 @@
 } /* initializeDictionaryDefaults */
 
 /*
- * This routine will attempt to load the XML dictionary, and on
- * failure, will call initializeDictionaryDefaults to load in
- * our static dictionary.
+ * This routine will attempt to load the XML dictionary if configured to.
+ * Otherwise, or if load fails, it will call initializeDictionaryDefaults
+ * to load in our static dictionary instead.
  */
 static void
 initializeDictionary(void)
@@ -930,12 +933,17 @@
    * loadXMLDictionary will be called.  This is one of the few times when
    * I think this is prettier than the nested if alternative.
    */
-  if (loadLibXML() ||
-	  (loadXMLDictionary() != 0)) {
-	/* Something failed.  Use the static dictionary */
-	report_failure("Diameter: Using static dictionary! (Unable to use XML)");
-	initializeDictionaryDefaults();
-  }
+   if (gbl_use_xml_dictionary) {
+      if (loadLibXML() || (loadXMLDictionary() != 0)) {
+	     /* Something failed.  Use the static dictionary */
+	     report_failure("Diameter: Using static dictionary! (Unable to use XML)");
+	     initializeDictionaryDefaults();
+      }
+   }
+   else {
+      initializeDictionaryDefaults();
+   }
+
 } /* initializeDictionary */
 
 
@@ -1207,6 +1215,8 @@
   /*
    * Only parse in dictionary if there are diameter packets to
    * dissect.
+   * TODO: should keep track of preference settings and free/reinitialize the
+   * dictionary when appropriate.
    */
   if (!initialized) {
 	  /* Read in our dictionary, if it exists. */
@@ -2224,7 +2234,7 @@
 	diameter_module = prefs_register_protocol(proto_diameter,
 											  proto_reg_handoff_diameter);
 	/* Register a configuration option for Diameter version */
-  prefs_register_enum_preference(diameter_module, "version", "Diameter version", "Standard version used for decoding", (gint *)&gbl_version, options, FALSE);
+	prefs_register_enum_preference(diameter_module, "version", "Diameter version", "Standard version used for decoding", (gint *)&gbl_version, options, FALSE);
 
 	prefs_register_uint_preference(diameter_module, "tcp.port",
 								   "Diameter TCP Port",
@@ -2258,6 +2268,16 @@
 	 */
 	g_free(default_diameterDictionary);
 
+	/*
+	 * Make use of the dictionary optional.  Avoids error popups if xml library
+	 * or dictionary file aren't available.
+	 */
+	prefs_register_bool_preference(diameter_module, "dictionary.use",
+	                               "Attempt to load/use Diameter XML Dictionary",
+	                               "Only attempt to load and use the Diameter XML\n"
+	                               "Dictionary when this option is selected",
+	                               &gbl_use_xml_dictionary);
+
 	/* Desegmentation */
 	prefs_register_bool_preference(diameter_module, "desegment",
                                    "Reassemble Diameter messages\nspanning multiple TCP segments",