extcap: Reduce number of scans and storage
authorRoland Knall <roland.knall@br-automation.com>
Tue, 21 Feb 2017 16:28:32 +0000 (17:28 +0100)
committerAnders Broman <a.broman58@gmail.com>
Wed, 22 Feb 2017 13:40:33 +0000 (13:40 +0000)
Reduce the number of storage arrays and the number
of necessary loads. Also include cleaner methods for
reloading the interfaces and cleanly reload if asked by
the overall system

Change-Id: I529465ec2593d40c955c6cdeaf3a85e3021c0596
Reviewed-on: https://code.wireshark.org/review/20230
Petri-Dish: Roland Knall <rknall@gmail.com>
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>
Reviewed-by: Roland Knall <rknall@gmail.com>
extcap.c
extcap.h
ui/qt/about_dialog.cpp
ui/qt/wireshark_application.cpp

index 1882ba0244413baa38495e7bc70ee173f4133b2e..42c2d6ee0d2d3c7e4c42234c0dab61e720af42ec 100644 (file)
--- a/extcap.c
+++ b/extcap.c
@@ -65,18 +65,17 @@ static HANDLE pipe_h = NULL;
 
 static void extcap_child_watch_cb(GPid pid, gint status, gpointer user_data);
 
-/* internal container, for all the extcap interfaces that have been found.
- * will be resetted by every call to extcap_interface_list() and is being
- * used in extcap_get_if_* as well as extcap_init_interfaces to ensure,
- * that only extcap interfaces are being given to underlying extcap programs
+/* internal container, for all the extcap executables that have been found.
+ * Will be resetted if extcap_clear_interfaces() is being explicitly called
+ * and is being used for printing information about all extcap interfaces found,
+ * as well as storing all sub-interfaces
  */
-static GHashTable *ifaces = NULL;
+static GHashTable * _loaded_interfaces = NULL;
 
-/* internal container, for all the extcap executables that have been found.
- * will be resetted by every call to extcap_interface_list() and is being
- * used for printing information about all extcap interfaces found
+/* Internal container, which maps each ifname to the tool providing it, for faster
+ * lookup.
  */
-static GHashTable *tools = NULL;
+static GHashTable * _tool_for_ifname = NULL;
 
 /* internal container, to map preference names to pointers that hold preference
  * values. These ensure that preferences can survive extcap if garbage
@@ -88,6 +87,29 @@ static GHashTable *extcap_prefs_dynamic_vals = NULL;
 typedef gboolean(*extcap_cb_t)(const gchar *extcap, const gchar *ifname, gchar *output, void *data,
                                gchar **err_str);
 
+static void extcap_load_interface_list(void);
+
+GHashTable *
+extcap_loaded_interfaces(void)
+{
+    if ( !_loaded_interfaces || g_hash_table_size(_loaded_interfaces) == 0 )
+        extcap_load_interface_list();
+
+    return _loaded_interfaces;
+}
+
+void
+extcap_clear_interfaces(void)
+{
+    if ( _loaded_interfaces )
+        g_hash_table_destroy(_loaded_interfaces);
+    _loaded_interfaces = NULL;
+
+    if ( _tool_for_ifname )
+        g_hash_table_destroy(_tool_for_ifname);
+    _tool_for_ifname = NULL;
+}
+
 guint extcap_count(void)
 {
     const char *dirname = get_extcap_dir();
@@ -122,12 +144,12 @@ guint extcap_count(void)
 static gboolean
 extcap_if_exists(const gchar *ifname)
 {
-    if (!ifname || !ifaces)
+    if (!ifname || !_tool_for_ifname)
     {
         return FALSE;
     }
 
-    if (g_hash_table_lookup(ifaces, ifname))
+    if (g_hash_table_lookup(_tool_for_ifname, ifname))
     {
         return TRUE;
     }
@@ -135,72 +157,56 @@ extcap_if_exists(const gchar *ifname)
     return FALSE;
 }
 
-static gboolean
-extcap_if_exists_for_extcap(const gchar *ifname, const gchar *extcap)
+static extcap_interface *
+extcap_find_interface_for_ifname(const gchar *ifname)
 {
-    extcap_interface *entry = (extcap_interface *)g_hash_table_lookup(ifaces, ifname);
+    extcap_interface * result = NULL;
 
-    if (entry && strcmp(entry->extcap_path, extcap) == 0)
-    {
-        return TRUE;
-    }
+    if ( !ifname || ! _tool_for_ifname || ! _loaded_interfaces )
+        return result;
 
-    return FALSE;
-}
+    gchar * extcap_util = (gchar *)g_hash_table_lookup(_tool_for_ifname, ifname);
+    if ( ! extcap_util )
+        return result;
 
-static gchar *
-extcap_if_executable(const gchar *ifname)
-{
-    extcap_interface *interface = (extcap_interface *)g_hash_table_lookup(ifaces, ifname);
-    return interface != NULL ? interface->extcap_path : NULL;
-}
+    extcap_info * element = (extcap_info *)g_hash_table_lookup(_loaded_interfaces, extcap_util);
+    if ( ! element )
+        return result;
 
-static gboolean
-extcap_if_add(extcap_interface *interface)
-{
-    if (g_hash_table_lookup(ifaces, interface->call))
+    GList * walker = element->interfaces;
+    while ( walker && walker->data && ! result )
     {
-        return FALSE;
-    }
-
-    g_hash_table_insert(ifaces, g_strdup(interface->call), interface);
-    return TRUE;
-}
+        extcap_interface * interface = (extcap_interface *)walker->data;
+        if ( g_strcmp0(interface->call, ifname) == 0 )
+        {
+            result = interface;
+            break;
+        }
 
-static void
-extcap_free_info(gpointer data)
-{
-    extcap_info *info = (extcap_info *)data;
+        walker = g_list_next ( walker );
+    }
 
-    g_free(info->basename);
-    g_free(info->full_path);
-    g_free(info->version);
-    g_free(info);
+    return result;
 }
 
-static void
-extcap_tool_add(const gchar *extcap, const extcap_interface *interface)
+static gboolean
+extcap_if_exists_for_extcap(const gchar *ifname, const gchar *extcap)
 {
-    char *toolname;
+    extcap_interface *entry = extcap_find_interface_for_ifname(ifname);
 
-    if (!extcap || !interface)
+    if (entry && strcmp(entry->extcap_path, extcap) == 0)
     {
-        return;
+        return TRUE;
     }
 
-    toolname = g_path_get_basename(extcap);
-
-    if (!g_hash_table_lookup(tools, toolname))
-    {
-        extcap_info *store = (extcap_info *)g_new0(extcap_info, 1);
-        store->version = g_strdup(interface->version);
-        store->full_path = g_strdup(extcap);
-        store->basename = g_strdup(toolname);
-
-        g_hash_table_insert(tools, g_strdup(toolname), store);
-    }
+    return FALSE;
+}
 
-    g_free(toolname);
+static gchar *
+extcap_if_executable(const gchar *ifname)
+{
+    extcap_interface *interface = extcap_find_interface_for_ifname(ifname);
+    return interface != NULL ? interface->extcap_path : NULL;
 }
 
 /* Note: args does not need to be NULL-terminated. */
@@ -381,82 +387,6 @@ static void extcap_free_interfaces(GList *interfaces)
     g_list_free(interfaces);
 }
 
-static gboolean interfaces_cb(const gchar *extcap, const gchar *ifname _U_, gchar *output, void *data,
-                              char **err_str _U_)
-{
-    GList **il = (GList **) data;
-    GList *interfaces = NULL, *walker = NULL, *tmp = NULL;
-    extcap_interface *int_iter = NULL;
-    if_info_t *if_info = NULL;
-
-    interfaces = extcap_parse_interfaces(output);
-
-    g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Extcap pipe %s ", extcap);
-
-    walker = interfaces;
-    char* help = NULL;
-    while (walker != NULL)
-    {
-        /* Whether the interface information needs to be preserved or not. */
-        gboolean preserve_interface = FALSE;
-
-        int_iter = (extcap_interface *)walker->data;
-        if (int_iter->if_type == EXTCAP_SENTENCE_INTERFACE && extcap_if_exists(int_iter->call))
-        {
-            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_WARNING, "Extcap interface \"%s\" is already provided by \"%s\" ",
-                  int_iter->call, (gchar *)extcap_if_executable(int_iter->call));
-            walker = g_list_next(walker);
-            continue;
-        }
-
-        if (int_iter->if_type == EXTCAP_SENTENCE_INTERFACE)
-            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "  Interface [%s] \"%s\" ",
-                  int_iter->call, int_iter->display);
-        else if (int_iter->if_type == EXTCAP_SENTENCE_EXTCAP)
-        {
-            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "  Extcap [%s] ", int_iter->call);
-            help = int_iter->help;
-        }
-
-        if (int_iter->if_type == EXTCAP_SENTENCE_INTERFACE)
-        {
-            if (il != NULL)
-            {
-                if_info = g_new0(if_info_t, 1);
-                if_info->name = g_strdup(int_iter->call);
-                if_info->friendly_name = g_strdup(int_iter->display);
-
-                if_info->type = IF_EXTCAP;
-
-                if_info->extcap = g_strdup(extcap);
-                *il = g_list_append(*il, if_info);
-            }
-
-            int_iter->extcap_path = g_strdup(extcap);
-            int_iter->help = g_strdup(help);
-            preserve_interface = extcap_if_add(int_iter);
-        }
-
-        /* Call for interfaces and tools alike. Multiple calls (because a tool has multiple
-         * interfaces) are handled internally */
-        extcap_tool_add(extcap, int_iter);
-
-        tmp = walker;
-        walker = g_list_next(walker);
-
-        /* If interface was added to ifaces hash list then the hash list will free
-         * the resources. Remove the interface from interfaces list so it won't be
-         * freed when exiting this function */
-        if (preserve_interface)
-        {
-            interfaces = g_list_delete_link(interfaces, tmp);
-        }
-    }
-    extcap_free_interfaces(interfaces);
-
-    return TRUE;
-}
-
 static gint
 if_info_compare(gconstpointer a, gconstpointer b)
 {
@@ -472,82 +402,60 @@ if_info_compare(gconstpointer a, gconstpointer b)
     return comp;
 }
 
-static void
-extcap_reload_interface_list(GList **retp, char **err_str)
-{
-    gchar *argv;
-
-    if (err_str != NULL)
-    {
-        *err_str = NULL;
-    }
-
-    /* ifaces is used as cache, do not destroy its contents when
-     * returning or no extcap interfaces can be queried for options */
-    if (ifaces == NULL)
-    {
-        ifaces = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, extcap_free_interface);
-    }
-    else
-    {
-        g_hash_table_remove_all(ifaces);
-    }
-
-    if (tools == NULL)
-    {
-        tools = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, extcap_free_info);
-    }
-    else
-    {
-        g_hash_table_remove_all(tools);
-    }
-
-    argv = g_strdup(EXTCAP_ARGUMENT_LIST_INTERFACES);
-
-    extcap_foreach(1, &argv, interfaces_cb, retp, err_str, NULL);
-
-    g_free(argv);
-}
-
 gchar *
 extcap_get_help_for_ifname(const char *ifname)
 {
-    extcap_interface *interface = (extcap_interface *)g_hash_table_lookup(ifaces, ifname);
+    extcap_interface *interface = extcap_find_interface_for_ifname(ifname);
     return interface != NULL ? interface->help : NULL;
 }
 
-GHashTable *
-extcap_tools_list(void)
-{
-    if (tools == NULL || g_hash_table_size(tools) == 0)
-    {
-        extcap_reload_interface_list(NULL, NULL);
-    }
-
-    return tools;
-}
-
 GList *
-append_extcap_interface_list(GList *list, char **err_str)
+append_extcap_interface_list(GList *list, char **err_str _U_)
 {
-    GList *ret = NULL;
-    GList *entry;
-    void *data;
+    GList *interface_list = NULL;
+    extcap_interface *data = NULL;
+    GList * ifutilkeys = NULL;
 
     /* Update the extcap interfaces and get a list of their if_infos */
-    extcap_reload_interface_list(&ret, err_str);
+    if ( !_loaded_interfaces || g_hash_table_size(_loaded_interfaces) == 0 )
+        extcap_load_interface_list();
+
+    ifutilkeys = g_hash_table_get_keys(_loaded_interfaces);
+    while ( ifutilkeys && ifutilkeys->data )
+    {
+        extcap_info * extinfo =
+                (extcap_info *) g_hash_table_lookup(_loaded_interfaces, (gchar *)ifutilkeys->data);
+        GList * walker = extinfo->interfaces;
+        while ( walker && walker->data )
+        {
+            interface_list = g_list_append(interface_list, walker->data);
+            walker = g_list_next(walker);
+        }
+
+        ifutilkeys = g_list_next(ifutilkeys);
+    }
 
     /* Sort that list */
-    ret = g_list_sort(ret, if_info_compare);
+    interface_list = g_list_sort(interface_list, if_info_compare);
 
     /* Append the interfaces in that list to the list we're handed. */
-    while (ret != NULL)
+    while (interface_list != NULL)
     {
-        entry = g_list_first(ret);
-        data = entry->data;
-        ret = g_list_delete_link(ret, entry);
-        list = g_list_append(list, data);
+        GList *entry = g_list_first(interface_list);
+        data = (extcap_interface *)entry->data;
+        interface_list = g_list_delete_link(interface_list, entry);
+
+        if_info_t * if_info = g_new0(if_info_t, 1);
+        if_info->name = g_strdup(data->call);
+        if_info->friendly_name = g_strdup(data->display);
+
+        if_info->type = IF_EXTCAP;
+
+        if_info->extcap = g_strdup(data->extcap_path);
+
+        list = g_list_append(list, if_info);
     }
+
     return list;
 }
 
@@ -571,12 +479,11 @@ void extcap_register_preferences(void)
         return;
     }
 
-    if (!ifaces || g_hash_table_size(ifaces) == 0)
-    {
-        extcap_reload_interface_list(NULL, NULL);
-    }
+    if ( !_loaded_interfaces || g_hash_table_size(_loaded_interfaces) == 0 )
+        extcap_load_interface_list();
 
-    g_hash_table_foreach(ifaces, extcap_register_preferences_callback, NULL);
+
+    g_hash_table_foreach(_tool_for_ifname, extcap_register_preferences_callback, NULL);
 }
 
 /**
@@ -586,19 +493,13 @@ void extcap_register_preferences(void)
 void extcap_cleanup(void)
 {
     if (extcap_prefs_dynamic_vals)
-    {
         g_hash_table_destroy(extcap_prefs_dynamic_vals);
-    }
 
-    if (ifaces)
-    {
-        g_hash_table_destroy(ifaces);
-    }
+    if (_loaded_interfaces)
+        g_hash_table_destroy(_loaded_interfaces);
 
-    if (tools)
-    {
-        g_hash_table_destroy(tools);
-    }
+    if (_tool_for_ifname)
+        g_hash_table_destroy(_tool_for_ifname);
 }
 
 void extcap_pref_store(extcap_arg *arg, const char *newval)
@@ -1395,6 +1296,159 @@ gboolean extcap_create_pipe(char **fifo)
     return TRUE;
 }
 
+/************* EXTCAP LOAD INTERFACE LIST ***************
+ *
+ * The following code handles loading and reloading the interface list. It is explicitly
+ * kept separate from the rest
+ */
+
+
+static void
+extcap_free_interface_info(gpointer data _U_)
+{
+    extcap_info *info = (extcap_info *)data;
+
+    g_free(info->basename);
+    g_free(info->full_path);
+    g_free(info->version);
+
+    extcap_free_interfaces(info->interfaces);
+
+    g_free(info);
+}
+
+static extcap_info *
+extcap_ensure_interface(const gchar * toolname)
+{
+    extcap_info * element = 0;
+
+    if ( ! toolname )
+        return element;
+
+    if ( ! _loaded_interfaces )
+        _loaded_interfaces = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, extcap_free_interface);
+
+    if ( ( element = (extcap_info *) g_hash_table_lookup(_loaded_interfaces, toolname ) ) == NULL )
+    {
+        g_hash_table_insert(_loaded_interfaces, g_strdup(toolname), g_new0(extcap_info, 1));
+        element = (extcap_info *) g_hash_table_lookup(_loaded_interfaces, toolname );
+    }
+
+    return element;
+}
+
+static gboolean cb_load_interfaces(const gchar *extcap, const gchar *ifname _U_, gchar *output _U_, void *data _U_,
+                              char **err_str _U_)
+{
+    GList *interfaces = NULL, *walker = NULL;
+    extcap_interface *int_iter = NULL;
+
+    GList * interface_keys = g_hash_table_get_keys(_loaded_interfaces);
+
+    interfaces = extcap_parse_interfaces(output);
+
+    g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Loading interface list for %s ", extcap);
+
+    walker = interfaces;
+    gchar* help = NULL;
+    while (walker != NULL)
+    {
+        int_iter = (extcap_interface *)walker->data;
+
+        gchar * toolname = g_path_get_basename(extcap);
+        extcap_info * element = extcap_ensure_interface(toolname);
+
+        if ( element == NULL )
+        {
+            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_ERROR, "Cannot store interface %s", extcap );
+            walker = g_list_next(walker);
+            continue;
+        }
+
+        g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Interface found %s\n", int_iter->call);
+
+        /* Help is not necessarily stored with the interface, but rather with the version string.
+         * As the version string allways comes in front of the interfaces, this ensures, that it get's
+         * properly stored with the interface */
+        if (int_iter->if_type == EXTCAP_SENTENCE_EXTCAP)
+        {
+            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "  Extcap [%s] ", int_iter->call);
+            /* Only initialize values if none are set. Need to check only one element here */
+            if ( ! element->version )
+            {
+                element->version = g_strdup(int_iter->version);
+                element->basename = g_strdup(toolname);
+                element->full_path = g_strdup(extcap);
+            }
+
+            help = int_iter->help;
+
+            walker = g_list_next(walker);
+            continue;
+        }
+
+        /* Only interface definitions will be parsed here. help is already set by the extcap element,
+         * which makes it necessary to have version in the list before the interfaces. This is normally
+         * the case by design, but could be changed by separating the information in extcap-base. */
+        if ( int_iter->if_type == EXTCAP_SENTENCE_INTERFACE )
+        {
+            if ( g_list_find(interface_keys, int_iter->call) )
+            {
+                g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_WARNING, "Extcap interface \"%s\" is already provided by \"%s\" ",
+                      int_iter->call, (gchar *)extcap_if_executable(int_iter->call));
+                walker = g_list_next(walker);
+                continue;
+            }
+
+            g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "  Interface [%s] \"%s\" ",
+                  int_iter->call, int_iter->display);
+
+            int_iter->extcap_path = g_strdup(extcap);
+
+            /* Only set the help, if it exists and no parsed help information is present */
+            if ( ! int_iter->help && help )
+                int_iter->help = g_strdup(help);
+
+            element->interfaces = g_list_append(element->interfaces, int_iter);
+            g_hash_table_insert(_tool_for_ifname, g_strdup(int_iter->call), g_strdup(toolname));
+        }
+
+        walker = g_list_next(walker);
+    }
+
+    return TRUE;
+}
+
+
+/* Handles loading of the interfaces.
+ *
+ * A list of interfaces can be obtained by calling \ref extcap_loaded_interfaces
+ */
+static void
+extcap_load_interface_list(void)
+{
+    gchar *argv;
+    gchar *error;
+
+    if (_loaded_interfaces == NULL)
+    {
+        _loaded_interfaces = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, extcap_free_interface_info);
+        /* Cleanup lookup table */
+        if ( _tool_for_ifname )
+        {
+            g_hash_table_remove_all(_tool_for_ifname);
+            _tool_for_ifname = 0;
+        }
+        _tool_for_ifname = g_hash_table_new(g_str_hash, g_str_equal);
+
+        argv = g_strdup(EXTCAP_ARGUMENT_LIST_INTERFACES);
+
+        extcap_foreach(1, &argv, cb_load_interfaces, NULL, &error, NULL);
+
+        g_free(argv);
+    }
+}
+
 /*
  * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
  *
index d066d9d2a9e25a5fc7b622a8b9320ff7b7c13cb4..9f398d1b5c8d4ab6e26104f02ff93e579f754667 100644 (file)
--- a/extcap.h
+++ b/extcap.h
@@ -55,6 +55,8 @@ typedef struct _extcap_info {
     gchar * basename;
     gchar * full_path;
     gchar * version;
+
+    GList * interfaces;
 } extcap_info;
 
 struct _extcap_arg;
@@ -83,9 +85,13 @@ append_extcap_interface_list(GList *list, char **err_str);
 gchar *
 extcap_get_help_for_ifname(const char *ifname);
 
-/* get a list of all available extcap tools */
+/* get a list of all available extcap executables and their interfaces */
 GHashTable *
-extcap_tools_list(void);
+extcap_loaded_interfaces(void);
+
+/* remove all loaded interfaces */
+void
+extcap_clear_interfaces(void);
 
 /* returns the configuration for the given interface name, or an
  * empty list, if no configuration has been found */
index e29bc0c2219f3751c6d1d60c9c8efcdccbcc388f..19faee4255004fb232fb45f15789705d207636c6 100644 (file)
@@ -120,11 +120,11 @@ const QString AboutDialog::plugins_scan()
     }
 
 #ifdef HAVE_EXTCAP
-    GHashTable * tools = extcap_tools_list();
+    GHashTable * tools = extcap_loaded_interfaces();
     if (tools && g_hash_table_size(tools) > 0) {
         QString short_file;
         GList * walker = g_list_first(g_hash_table_get_keys(tools));
-        while (walker) {
+        while (walker && walker->data) {
             extcap_info * tool = (extcap_info *)g_hash_table_lookup(tools, walker->data);
             if (tool) {
                 short_file = fontMetrics().elidedText(tool->full_path, Qt::ElideMiddle, one_em*22);
index f75a46099cb8a4f30910495011aee8239def9b51..e301cc2bf9827d9f53ffb817c9cbc6d7665e5b29 100644 (file)
@@ -57,6 +57,9 @@
 #include "log.h"
 #include "recent_file_status.h"
 
+#ifdef HAVE_EXTCAP
+#include "extcap.h"
+#endif
 #ifdef HAVE_LIBPCAP
 #include <caputils/iface_monitor.h>
 #endif
@@ -1057,6 +1060,10 @@ void WiresharkApplication::ifChangeEventsAvailable()
 
 void WiresharkApplication::refreshLocalInterfaces()
 {
+#ifdef HAVE_EXTCAP
+    extcap_clear_interfaces();
+#endif
+
 #ifdef HAVE_LIBPCAP
     /*
      * Reload the local interface list.