Fix various memleaks

Found by starting Wireshark within an empty profile, opening
Preferences, search for Protocol "IEEE 802.11" (because it has radio
buttons), then close everything again.

Many fixes are trivial, but the various recent_read_* functions in
recent.c were changed to return a boolean such that the result can
always be checked even if errno==0.

QButtonGroup leak was hinted by Clang Static Analyzer, all other
memleaks were found using ASAN/LSan.

Change-Id: Ia73f5d4c09d92f22e72377be59e23342f8ad7211
Reviewed-on: https://code.wireshark.org/review/10776
Reviewed-by: Michael Mann <mmann78@netscape.net>
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>

diff --git a/capchild/capture_ifinfo.c b/capchild/capture_ifinfo.c
index 510f52954983391aa78fdf60c79131881170555b..30cb7d1b0ea3afe7af4233f61fdbc21d042fe52c 100644 (file)
--- a/capchild/capture_ifinfo.c
+++ b/capchild/capture_ifinfo.c
@@ -323,6 +323,7 @@ capture_get_if_capabilities(const gchar *ifname, gboolean monitor_mode,
             data_link_info->description = g_strdup(lt_parts[2]);
         else
             data_link_info->description = NULL;
+        g_strfreev(lt_parts);
 
         linktype_list = g_list_append(linktype_list, data_link_info);
     }

diff --git a/caputils/capture-pcap-util.c b/caputils/capture-pcap-util.c
index a648eff0bad41e276f73b4f7dcbb3f7b9a9a3174..9133a3dca1f88076e3b98301c2efc2b6422aa2d2 100644 (file)
--- a/caputils/capture-pcap-util.c
+++ b/caputils/capture-pcap-util.c
@@ -625,6 +625,7 @@ free_linktype_cb(gpointer data, gpointer user_data _U_)
 
        g_free(linktype_info->name);
        g_free(linktype_info->description);
+       g_free(linktype_info);
 }
 
 void

diff --git a/epan/prefs.c b/epan/prefs.c
index a9eef65aebabde5ae49a6cfff22b304a8dc3dea1..127c4846995e339eea81ceaeab2e90bc1b664600 100644 (file)
--- a/epan/prefs.c
+++ b/epan/prefs.c
@@ -1527,6 +1527,7 @@ column_hidden_to_str_cb(pref_t* pref, gboolean default_val)
                 g_string_append (cols_hidden, ",");
             g_string_append (cols_hidden, prefs_fmt);
         }
+        g_free(prefs_fmt);
         clp = clp->next;
     }
 

diff --git a/ui/capture.c b/ui/capture.c
index 8113c67166beda3f607b4a478618053b339dbb8f..f553fa8975a6b9aac856e37d82021fdc34f5634f 100644 (file)
--- a/ui/capture.c
+++ b/ui/capture.c
@@ -764,6 +764,7 @@ capture_stat_stop(if_stat_cache_t *sc) {
     g_free(sc_item->name);
     g_free(sc_item);
   }
+  g_list_free(sc->cache_list);
   g_free(sc);
 }
 

diff --git a/ui/gtk/main.c b/ui/gtk/main.c
index 721f942e5368d65bab5078b0db3f20497c5135cd..5870daef0bd12a2af6361024c840746c0338ab02 100644 (file)
--- a/ui/gtk/main.c
+++ b/ui/gtk/main.c
@@ -2441,11 +2441,11 @@ DIAG_ON(cast-qual)
     /* Only the static part of it will be read, as we don't have the gui now to fill the */
     /* recent lists which is done in the dynamic part. */
     /* We have to do this already here, so command line parameters can overwrite these values. */
-    recent_read_profile_static(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_profile_static(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
                       "Could not open recent file\n\"%s\": %s.",
                       rf_path, g_strerror(rf_open_errno));
+        g_free(rf_path);
     }
 
     if (recent.gui_fileopen_remembered_dir &&
@@ -3097,11 +3097,11 @@ DIAG_ON(cast-qual)
     create_main_window(pl_size, tv_size, bv_size, prefs_p);
 
     /* Read the dynamic part of the recent file, as we have the gui now ready for it. */
-    recent_read_dynamic(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_dynamic(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
                       "Could not open recent file\n\"%s\": %s.",
                       rf_path, g_strerror(rf_open_errno));
+        g_free(rf_path);
     }
 
     color_filters_enable(recent.packet_list_colorize);
@@ -3880,11 +3880,11 @@ void change_configuration_profile (const gchar *profile_name)
 
     (void) read_configuration_files (&gdp_path, &dp_path);
 
-    recent_read_profile_static(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_profile_static(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
             "Could not open common recent file\n\"%s\": %s.",
             rf_path, g_strerror(rf_open_errno));
+        g_free(rf_path);
     }
     if (recent.gui_fileopen_remembered_dir &&
         test_for_directory(recent.gui_fileopen_remembered_dir) == EISDIR) {

diff --git a/ui/iface_lists.c b/ui/iface_lists.c
index 8c29dd357e45fd3d34ab264e04cc408e6d49a361..ba07c3eb7eb0450f5c7a0befd67a1328e849089d 100644 (file)
--- a/ui/iface_lists.c
+++ b/ui/iface_lists.c
@@ -63,7 +63,7 @@ void
 scan_local_interfaces(void (*update_cb)(void))
 {
     GList             *if_entry, *lt_entry, *if_list;
-    if_info_t         *if_info, *temp;
+    if_info_t         *if_info, temp;
     char              *if_string;
     gchar             *descr;
     if_capabilities_t *caps=NULL;
@@ -130,14 +130,14 @@ scan_local_interfaces(void (*update_cb)(void))
         }
         device.hidden = FALSE;
         device.locked = FALSE;
-        temp = (if_info_t *)g_malloc0(sizeof(if_info_t));
-        temp->name = g_strdup(if_info->name);
-        temp->friendly_name = g_strdup(if_info->friendly_name);
-        temp->vendor_description = g_strdup(if_info->vendor_description);
-        temp->loopback = if_info->loopback;
-        temp->type = if_info->type;
+        memset(&temp, 0, sizeof(temp));
+        temp.name = g_strdup(if_info->name);
+        temp.friendly_name = g_strdup(if_info->friendly_name);
+        temp.vendor_description = g_strdup(if_info->vendor_description);
+        temp.loopback = if_info->loopback;
+        temp.type = if_info->type;
 #ifdef HAVE_EXTCAP
-        temp->extcap = g_strdup(if_info->extcap);
+        temp.extcap = g_strdup(if_info->extcap);
 #endif
         /* Is this interface hidden and, if so, should we include it anyway? */
 
@@ -217,7 +217,7 @@ scan_local_interfaces(void (*update_cb)(void))
                 temp_addr = NULL;
             }
             if (temp_addr) {
-                temp->addrs = g_slist_append(temp->addrs, temp_addr);
+                temp.addrs = g_slist_append(temp.addrs, temp_addr);
             }
         }
 #ifdef HAVE_PCAP_REMOTE
@@ -274,7 +274,7 @@ scan_local_interfaces(void (*update_cb)(void))
         device.addresses = g_strdup(ip_str->str);
         device.no_addresses = ips;
         device.local = TRUE;
-        device.if_info = *temp;
+        device.if_info = temp;
         device.last_packets = 0;
         if (!capture_dev_user_pmode_find(if_info->name, &device.pmode)) {
             device.pmode = global_capture_opts.default_options.promisc_mode;

diff --git a/ui/qt/main_welcome.cpp b/ui/qt/main_welcome.cpp
index 27f6aaeb7b078728fc5e071ebe065637a1b25e82..492a4dfbb4461fcc59881897dda40c6d4efd4f96 100644 (file)
--- a/ui/qt/main_welcome.cpp
+++ b/ui/qt/main_welcome.cpp
@@ -176,6 +176,11 @@ MainWelcome::MainWelcome(QWidget *parent) :
     splash_overlay_ = new SplashOverlay(this);
 }
 
+MainWelcome::~MainWelcome()
+{
+    delete welcome_ui_;
+}
+
 InterfaceTree *MainWelcome::getInterfaceTree()
 {
     return welcome_ui_->interfaceTree;

diff --git a/ui/qt/main_welcome.h b/ui/qt/main_welcome.h
index 6e70aaef1927fe4514e398662f208909116fec84..97d66c0f538068339c6e4ac9305c9f5364ec4a63 100644 (file)
--- a/ui/qt/main_welcome.h
+++ b/ui/qt/main_welcome.h
@@ -40,6 +40,7 @@ class MainWelcome : public QFrame
     Q_OBJECT
 public:
     explicit MainWelcome(QWidget *parent = 0);
+    virtual ~MainWelcome();
     InterfaceTree *getInterfaceTree();
 
 protected:

diff --git a/ui/qt/module_preferences_scroll_area.cpp b/ui/qt/module_preferences_scroll_area.cpp
index 4fabd71044ed7bf3f533e2c7790a7d7799c5fb70..3144133d4baf44efc84c595374137efc663f9ee3 100644 (file)
--- a/ui/qt/module_preferences_scroll_area.cpp
+++ b/ui/qt/module_preferences_scroll_area.cpp
@@ -114,7 +114,7 @@ pref_show(pref_t *pref, gpointer layout_ptr)
             QLabel *label = new QLabel(pref->title);
             label->setToolTip(tooltip);
             vb->addWidget(label);
-            QButtonGroup *enum_bg = new QButtonGroup();
+            QButtonGroup *enum_bg = new QButtonGroup(vb);
             for (ev = pref->info.enum_info.enumvals; ev && ev->description; ev++) {
                 QRadioButton *enum_rb = new QRadioButton(title_to_shortcut(ev->description));
                 enum_rb->setToolTip(tooltip);

diff --git a/ui/qt/wireshark_application.cpp b/ui/qt/wireshark_application.cpp
index 3dc82971430efc659908813465397c7bca26d8d4..3d5f983d7f39f623436a2bdf53fbc37d48fa00e2 100644 (file)
--- a/ui/qt/wireshark_application.cpp
+++ b/ui/qt/wireshark_application.cpp
@@ -342,11 +342,11 @@ void WiresharkApplication::setConfigurationProfile(const gchar *profile_name)
 
     (void) readConfigurationFiles (&gdp_path, &dp_path);
 
-    recent_read_profile_static(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_profile_static(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
             "Could not open common recent file\n\"%s\": %s.",
             rf_path, g_strerror(rf_open_errno));
+        g_free(rf_path);
     }
     if (recent.gui_fileopen_remembered_dir &&
         test_for_directory(recent.gui_fileopen_remembered_dir) == EISDIR) {

diff --git a/ui/recent.c b/ui/recent.c
index 2633ac92cfffd7008d999a5b8bbf6755422a6d80..82666ffff29983198cc6361296a3f58a3edb0935 100644 (file)
--- a/ui/recent.c
+++ b/ui/recent.c
@@ -1208,7 +1208,7 @@ recent_set_arg(char *prefarg)
 
 
 /* opens the user's recent common file and read the first part */
-void
+gboolean
 recent_read_static(char **rf_path_return, int *rf_errno_return)
 {
   char       *rf_path;
@@ -1241,8 +1241,6 @@ recent_read_static(char **rf_path_return, int *rf_errno_return)
     read_prefs_file(rf_path, rf, read_set_recent_common_pair_static, NULL);
 
     fclose(rf);
-    g_free(rf_path);
-    rf_path = NULL;
   } else {
     /* We failed to open it.  If we failed for some reason other than
        "it doesn't exist", return the errno and the pathname, so our
@@ -1250,14 +1248,17 @@ recent_read_static(char **rf_path_return, int *rf_errno_return)
     if (errno != ENOENT) {
       *rf_errno_return = errno;
       *rf_path_return = rf_path;
+      return FALSE;
     }
   }
+  g_free(rf_path);
+  return TRUE;
 }
 
 
 
 /* opens the user's recent file and read the first part */
-void
+gboolean
 recent_read_profile_static(char **rf_path_return, int *rf_errno_return)
 {
   char       *rf_path, *rf_common_path;
@@ -1322,8 +1323,6 @@ recent_read_profile_static(char **rf_path_return, int *rf_errno_return)
       fclose(rf);
     }
     g_free(rf_common_path);
-    g_free(rf_path);
-    rf_path = NULL;
   } else {
     /* We failed to open it.  If we failed for some reason other than
        "it doesn't exist", return the errno and the pathname, so our
@@ -1331,12 +1330,15 @@ recent_read_profile_static(char **rf_path_return, int *rf_errno_return)
     if (errno != ENOENT) {
       *rf_errno_return = errno;
       *rf_path_return = rf_path;
+      return FALSE;
     }
   }
+  g_free(rf_path);
+  return TRUE;
 }
 
 /* opens the user's recent file and read it out */
-void
+gboolean
 recent_read_dynamic(char **rf_path_return, int *rf_errno_return)
 {
   char       *rf_path;
@@ -1361,8 +1363,6 @@ recent_read_dynamic(char **rf_path_return, int *rf_errno_return)
     dfilter_combo_add_empty();
 #endif
     fclose(rf);
-    g_free(rf_path);
-    rf_path = NULL;
   } else {
     /* We failed to open it.  If we failed for some reason other than
        "it doesn't exist", return the errno and the pathname, so our
@@ -1370,8 +1370,11 @@ recent_read_dynamic(char **rf_path_return, int *rf_errno_return)
     if (errno != ENOENT) {
       *rf_errno_return = errno;
       *rf_path_return = rf_path;
+      return FALSE;
     }
   }
+  g_free(rf_path);
+  return TRUE;
 }
 
 gint

diff --git a/ui/recent.h b/ui/recent.h
index ef5f357f07ef7eefb3107f1a16640ab8a5c9481d..766f5be4737282f7b1cf6ed6dd8ccf06c67fb5a1 100644 (file)
--- a/ui/recent.h
+++ b/ui/recent.h
@@ -123,22 +123,25 @@ extern gboolean write_profile_recent(void);
  *
  * @param rf_path_return path to recent file if function failed
  * @param rf_errno_return if failed
+ * @return TRUE if succeeded, FALSE if failed (check parameters for reason).
  */
-extern void recent_read_static(char **rf_path_return, int *rf_errno_return);
+extern gboolean recent_read_static(char **rf_path_return, int *rf_errno_return);
 
 /** Read profile recent settings file (static part).
  *
  * @param rf_path_return path to recent file if function failed
  * @param rf_errno_return if failed
+ * @return TRUE if succeeded, FALSE if failed (check parameters for reason).
  */
-extern void recent_read_profile_static(char **rf_path_return, int *rf_errno_return);
+extern gboolean recent_read_profile_static(char **rf_path_return, int *rf_errno_return);
 
 /** Read recent settings file (dynamic part).
  *
  * @param rf_path_return path to recent file if function failed
  * @param rf_errno_return if failed
+ * @return TRUE if succeeded, FALSE if failed (check parameters for reason).
  */
-extern void recent_read_dynamic(char **rf_path_return, int *rf_errno_return);
+extern gboolean recent_read_dynamic(char **rf_path_return, int *rf_errno_return);
 
 /**
  * Given a -o command line string, parse it and set the recent value in

diff --git a/wireshark-qt.cpp b/wireshark-qt.cpp
index 6674631078b8f8b762cd8d716c3b4077af58a265..acfa7083a34081d3fecdb68028e0f34892e355a5 100644 (file)
--- a/wireshark-qt.cpp
+++ b/wireshark-qt.cpp
@@ -740,11 +740,11 @@ DIAG_ON(cast-qual)
 
     /* Read the profile independent recent file.  We have to do this here so we can */
     /* set the profile before it can be set from the command line parameter */
-    recent_read_static(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_static(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
                       "Could not open common recent file\n\"%s\": %s.",
                       rf_path, strerror(rf_open_errno));
+        g_free(rf_path);
     }
 
     /* Init the "Open file" dialog directory */
@@ -754,11 +754,11 @@ DIAG_ON(cast-qual)
     /* Only the static part of it will be read, as we don't have the gui now to fill the */
     /* recent lists which is done in the dynamic part. */
     /* We have to do this already here, so command line parameters can overwrite these values. */
-    recent_read_profile_static(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_profile_static(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
                       "Could not open recent file\n\"%s\": %s.",
                       rf_path, g_strerror(rf_open_errno));
+        g_free(rf_path);
     }
 
     // Initialize our language
@@ -783,11 +783,11 @@ DIAG_ON(cast-qual)
     /* Only the static part of it will be read, as we don't have the gui now to fill the */
     /* recent lists which is done in the dynamic part. */
     /* We have to do this already here, so command line parameters can overwrite these values. */
-    recent_read_profile_static(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_profile_static(&rf_path, &rf_open_errno)) {
       simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
             "Could not open recent file\n\"%s\": %s.",
             rf_path, g_strerror(rf_open_errno));
+      g_free(rf_path);
     }
 
     if (recent.gui_fileopen_remembered_dir &&
@@ -1347,11 +1347,11 @@ DIAG_ON(cast-qual)
 
     /* Read the dynamic part of the recent file, as we have the gui now ready for
        it. */
-    recent_read_dynamic(&rf_path, &rf_open_errno);
-    if (rf_path != NULL && rf_open_errno != 0) {
+    if (!recent_read_dynamic(&rf_path, &rf_open_errno)) {
         simple_dialog(ESD_TYPE_WARN, ESD_BTN_OK,
                       "Could not open recent file\n\"%s\": %s.",
                       rf_path, g_strerror(rf_open_errno));
+        g_free(rf_path);
     }
 
     color_filters_enable(recent.packet_list_colorize);