UI: Make sure we always have interface statistics.

Ensure that capture_stat_start always returns a non-NULL if_stat_cache_t
pointer. This keeps InterfaceTreeModel::updateStatistic from repeatedly
running dumpcap when we're unable to gather statistics, e.g. when we
don't have capture permissions.

Bug: 14284
Change-Id: Id408714a934abab2abdee1d4bb5e4bed872af016
Reviewed-on: https://code.wireshark.org/review/31038
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>

3 files changed, 11 insertions, 11 deletions

diff --git a/ui/capture.c b/ui/capture.c
index 48100fceb3..2478733b0f 100644
--- a/ui/capture.c
+++ b/ui/capture.c
@@ -765,11 +765,14 @@ capture_stat_start(capture_options *capture_opts)
     int stat_fd;
     ws_process_id fork_child;
     gchar *msg;
-    if_stat_cache_t *sc = NULL;
+    if_stat_cache_t *sc = g_new0(if_stat_cache_t, 1);
     if_stat_cache_item_t *sc_item;
     guint i;
     interface_t *device;
 
+    sc->stat_fd = -1;
+    sc->fork_child = WS_INVALID_PID;
+
     /* Fire up dumpcap. */
     /*
      * XXX - on systems with BPF, the number of BPF devices limits the
@@ -790,10 +793,8 @@ capture_stat_start(capture_options *capture_opts)
      * counts might not always be a good idea.
      */
     if (sync_interface_stats_open(&stat_fd, &fork_child, &msg, NULL) == 0) {
-        sc = (if_stat_cache_t *)g_malloc(sizeof(if_stat_cache_t));
         sc->stat_fd = stat_fd;
         sc->fork_child = fork_child;
-        sc->cache_list = NULL;
 
         /* Initialize the cache */
         for (i = 0; i < capture_opts->all_ifaces->len; i++) {
@@ -821,8 +822,9 @@ capture_stat_cache_update(if_stat_cache_t *sc)
     GList *sc_entry;
     if_stat_cache_item_t *sc_item;
 
-    if (!sc)
+    if (!sc || sc->fork_child == WS_INVALID_PID) {
         return;
+    }
 
     while (sync_pipe_gets_nonblock(sc->stat_fd, stat_line, MAX_STAT_LINE_LEN) > 0) {
         g_strstrip(stat_line);
@@ -849,7 +851,7 @@ capture_stats(if_stat_cache_t *sc, char *ifname, struct pcap_stat *ps)
     GList *sc_entry;
     if_stat_cache_item_t *sc_item;
 
-    if (!sc || !ifname || !ps) {
+    if (!sc || sc->fork_child == WS_INVALID_PID || !ifname || !ps) {
         return FALSE;
     }
 
@@ -872,8 +874,9 @@ capture_stat_stop(if_stat_cache_t *sc)
     int ret;
     gchar *msg;
 
-    if (!sc)
+    if (!sc || sc->fork_child == WS_INVALID_PID) {
         return;
+    }
 
     ret = sync_interface_stats_close(&sc->stat_fd, &sc->fork_child, &msg);
     if (ret == -1) {
diff --git a/ui/capture.h b/ui/capture.h
index ff0e63539c..d4de9efe09 100644
--- a/ui/capture.h
+++ b/ui/capture.h
@@ -78,7 +78,7 @@ typedef struct if_stat_cache_s if_stat_cache_t;
  * @param capture_opts A structure containing options for the capture.
  * @return A pointer to the statistics state data.
  */
-extern if_stat_cache_t * capture_stat_start(capture_options *capture_opts);
+extern WS_RETNONNULL if_stat_cache_t * capture_stat_start(capture_options *capture_opts);
 
 /**
  * Fetch capture statistics, similar to pcap_stats().
diff --git a/ui/qt/models/interface_tree_model.cpp b/ui/qt/models/interface_tree_model.cpp
index ae44e131b2..ec4480cf02 100644
--- a/ui/qt/models/interface_tree_model.cpp
+++ b/ui/qt/models/interface_tree_model.cpp
@@ -405,7 +405,6 @@ void InterfaceTreeModel::stopStatistic()
 void InterfaceTreeModel::updateStatistic(unsigned int idx)
 {
 #ifdef HAVE_LIBPCAP
-    guint diff;
     if ( ! global_capture_opts.all_ifaces || global_capture_opts.all_ifaces->len <= (guint) idx )
         return;
 
@@ -420,12 +419,10 @@ void InterfaceTreeModel::updateStatistic(unsigned int idx)
         // We crash (on macOS at least) if we try to do this from ::showEvent.
         stat_cache_ = capture_stat_start(&global_capture_opts);
     }
-    if ( !stat_cache_ )
-        return;
 
     struct pcap_stat stats;
+    unsigned diff = 0;
 
-    diff = 0;
     if ( capture_stats(stat_cache_, device->name, &stats) )
     {
         if ( (int)(stats.ps_recv - device->last_packets) >= 0 )