2007/3/30, Jeff Morriss <[EMAIL PROTECTED]>:



Peter Johansson wrote:
>     I compiled Wireshark with HAVE_AIRPDCAP by mistake (since I do not
>     have AirPcap). This leads to a runtime problem however. When
>     choosing "options" from the "Capture interfaces" dialog, I receive a
>     modal dialogue with an OK button with a textual description that is
>     only garbage (uninitialized memory).
>
>     The provided patch adds a new error - AIRPCAP_NOT_LOADED (2) code to
>     the airpcap loader that also adds the text "AirPcap was expected to
>     be loaded but is not" to the modal dialogue instead of the
>     uninitialized string.

Hmm, I think you found the problem of bug 1377:

http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=1377

Having a quick look, though, I am a bit confused because it doesn't
appear anybody ever allocates any space for 'err_str' before calling
though maybe I'm missing it.  Anyway I don't really have time to dig
further for the moment.

Another thing that needs a look is it appears that it's normal that
AirPcap is compiled in (I didn't change my setup and my Windoze builds
say "with Airpcap") though in that case I'm not sure it should be
complaining at all if it doesn't find an AirPcap device or whatever.


After having read the description for bug 1377 I agree. This patch fixes
that problem.

err_str should not be allocated prior to calling
get_airpcap_interface_list(&err, &err_str). err_str gets assigned a pointer
to allocated memory (g_strdup) in the called function that gets freed when
returning from the function.

Thank you for having me review my changes once more. It turns out that my
original change will make the code call g_free for the newly added
statically allocated error description which would be a new error.
I have attached a new set of files to be supplied as a solution for bug
1377. This time my changes allocate the error description using g_strdup
just like the rest of the code, making it possible to call g_free upon
return from the get_airpcap_interface_list(...) function.

I was also wondering how to handle this case, should it not produce a
dialogue at all? I am unsure. If a dialogue is not displayed, the user will
never understand why the AirPcap devices do not show up if AirPcap is not
loaded but Wireshark is compiled with support for it. On the other hand, I
guess most users do not have AirPcap, which means that it would be annoying
to get the dialogue every time when entering the options dialogue.

Therefor I have changed the code in airpcap_loader.c even further to produce
the modal dialogue stating that AirPcap is not loaded only once per
Wireshark session. Hence it will not get displayed more than once unless the
user restarts Wireshark. Unfortunately I had to make changes to more files
than I would have wanted to be able to do this.

Is that good enough?

Regards, Peter
Index: C:/wireshark-win32-libs/airpcap_loader.c
===================================================================
--- C:/wireshark-win32-libs/airpcap_loader.c    (revision 21279)
+++ C:/wireshark-win32-libs/airpcap_loader.c    (working copy)
@@ -1138,7 +1138,11 @@
     char errbuf[PCAP_ERRBUF_SIZE];
 
     if (!AirpcapLoaded)
-       return il;
+    {
+        *err = AIRPCAP_NOT_LOADED;
+        *err_str = g_strdup("AirPcap was expected to be loaded but is not");
+        return il;
+    }
 
     if (!g_PAirpcapGetDeviceList(&devsList, errbuf))
     {
Index: C:/wireshark-win32-libs/airpcap_loader.h
===================================================================
--- C:/wireshark-win32-libs/airpcap_loader.h    (revision 21279)
+++ C:/wireshark-win32-libs/airpcap_loader.h    (working copy)
@@ -33,6 +33,7 @@
 /* Error values from "get_airpcap_interface_list()". */
 #define        CANT_GET_AIRPCAP_INTERFACE_LIST 0       /* error getting list */
 #define        NO_AIRPCAP_INTERFACES_FOUND     1       /* list is empty */
+#define        AIRPCAP_NOT_LOADED      2       /* AirPcap not loaded */
 
 #define AIRPCAP_CHANNEL_ANY_NAME "ANY"
 
Index: C:/wireshark-win32-libs/gtk/capture_dlg.c
===================================================================
--- C:/wireshark-win32-libs/gtk/capture_dlg.c   (revision 21279)
+++ C:/wireshark-win32-libs/gtk/capture_dlg.c   (working copy)
@@ -639,7 +639,9 @@
   decryption_cm = OBJECT_GET_DATA(airpcap_tb,AIRPCAP_TOOLBAR_DECRYPTION_KEY);
   update_decryption_mode_list(decryption_cm);
 
-  if (airpcap_if_list == NULL && err == CANT_GET_AIRPCAP_INTERFACE_LIST) {
+  if (airpcap_if_list == NULL &&
+      (err == CANT_GET_AIRPCAP_INTERFACE_LIST ||
+       err == AIRPCAP_NOT_LOADED)) {
     simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", err_str);
     g_free(err_str);
   }
Index: C:/wireshark-win32-libs/gtk/capture_if_dlg.c
===================================================================
--- C:/wireshark-win32-libs/gtk/capture_if_dlg.c        (revision 21279)
+++ C:/wireshark-win32-libs/gtk/capture_if_dlg.c        (working copy)
@@ -459,7 +459,9 @@
   decryption_cm = OBJECT_GET_DATA(airpcap_tb,AIRPCAP_TOOLBAR_DECRYPTION_KEY);
   update_decryption_mode_list(decryption_cm);
 
-  if (airpcap_if_list == NULL && err == CANT_GET_AIRPCAP_INTERFACE_LIST) {
+  if (airpcap_if_list == NULL &&
+      (err == CANT_GET_AIRPCAP_INTERFACE_LIST ||
+       err == AIRPCAP_NOT_LOADED)) {
     simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", err_str);
     g_free(err_str);
   }
Index: C:/wireshark-win32-libs/gtk/main.c
===================================================================
--- C:/wireshark-win32-libs/gtk/main.c  (revision 21279)
+++ C:/wireshark-win32-libs/gtk/main.c  (working copy)
@@ -2180,7 +2180,9 @@
     /* load the airpcap interfaces */
     airpcap_if_list = get_airpcap_interface_list(&err, &err_str);
 
-    if (airpcap_if_list == NULL && err == CANT_GET_AIRPCAP_INTERFACE_LIST) {
+    if (airpcap_if_list == NULL &&
+        (err == CANT_GET_AIRPCAP_INTERFACE_LIST ||
+         err == AIRPCAP_NOT_LOADED)) {
       simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", err_str);
       g_free(err_str);
     }
_______________________________________________
Wireshark-dev mailing list
Wireshark-dev@wireshark.org
http://www.wireshark.org/mailman/listinfo/wireshark-dev

Reply via email to