Qt: fix alloc-dealloc-mismatch while adding named pipe

ManageInterfacesDialog::on_addPipe_clicked uses g_new0 to create an
"interface_t" instance, but InterfaceTreeCacheModel uses qDeleteAll
which results in ASAN reporting "alloc-dealloc-mismatch (malloc vs
operator delete)".

To fix this, remove the dynamic allocation and make
InterfaceTreeCacheModel store the instance internally.

Change-Id: I9426dfc88d0a54a889bbbc9cf336c0a6af76920e
Reviewed-on: https://code.wireshark.org/review/22410
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Roland Knall <rknall@gmail.com>

3 files changed, 30 insertions, 34 deletions

diff --git a/ui/qt/interface_tree_cache_model.cpp b/ui/qt/interface_tree_cache_model.cpp
index 5447f80332..3b9f6d56b6 100644
--- a/ui/qt/interface_tree_cache_model.cpp
+++ b/ui/qt/interface_tree_cache_model.cpp
@@ -62,11 +62,7 @@ InterfaceTreeCacheModel::~InterfaceTreeCacheModel()
 {
 #ifdef HAVE_LIBPCAP
     /* This list should only exist, if the dialog is closed, without calling save first */
-    if ( newDevices.size() > 0 )
-    {
-        qDeleteAll(newDevices);
-        newDevices.clear();
-    }
+    newDevices.clear();
 #endif
 
     delete storage;
@@ -95,7 +91,7 @@ void InterfaceTreeCacheModel::reset(int row)
 
 void InterfaceTreeCacheModel::saveNewDevices()
 {
-    QList<interface_t *>::const_iterator it = newDevices.constBegin();
+    QList<interface_t>::const_iterator it = newDevices.constBegin();
     /* idx is used for iterating only over the indices of the new devices. As all new
      * devices are stored with an index higher then sourceModel->rowCount(), we start
      * only with those storage indices.
@@ -103,7 +99,7 @@ void InterfaceTreeCacheModel::saveNewDevices()
      * have storage, which will lead to that device not being stored in global_capture_opts */
     for (int idx = sourceModel->rowCount(); it != newDevices.constEnd(); ++it, idx++)
     {
-        interface_t * device = (interface_t *)(*it);
+        interface_t *device = const_cast<interface_t *>(&(*it));
         bool useDevice = false;
 
         QMap<InterfaceTreeColumns, QVariant> * dataField = storage->value(idx, 0);
@@ -143,7 +139,6 @@ void InterfaceTreeCacheModel::saveNewDevices()
         delete dataField;
     }
 
-    qDeleteAll(newDevices);
     newDevices.clear();
 }
 
@@ -321,9 +316,9 @@ bool InterfaceTreeCacheModel::changeIsAllowed(InterfaceTreeColumns col) const
 }
 
 #ifdef HAVE_LIBPCAP
-interface_t * InterfaceTreeCacheModel::lookup(const QModelIndex &index) const
+const interface_t * InterfaceTreeCacheModel::lookup(const QModelIndex &index) const
 {
-    interface_t * result = 0;
+    const interface_t * result = 0;
 
     if ( ! index.isValid() )
         return result;
@@ -336,7 +331,7 @@ interface_t * InterfaceTreeCacheModel::lookup(const QModelIndex &index) const
     {
         idx = idx - global_capture_opts.all_ifaces->len;
         if ( idx < newDevices.size() )
-            result = newDevices[idx];
+            result = &newDevices[idx];
     }
     else
     {
@@ -355,7 +350,7 @@ bool InterfaceTreeCacheModel::isAllowedToBeEdited(const QModelIndex &index) cons
     Q_UNUSED(index)
 
 #ifdef HAVE_LIBPCAP
-    interface_t * device = lookup(index);
+    const interface_t * device = lookup(index);
     if ( device == 0 )
         return false;
 
@@ -383,7 +378,7 @@ bool InterfaceTreeCacheModel::isAvailableField(const QModelIndex &index) const
     Q_UNUSED(index)
 
 #ifdef HAVE_LIBPCAP
-    interface_t * device = lookup(index);
+    const interface_t * device = lookup(index);
 
     if ( device == 0 )
         return false;
@@ -502,7 +497,7 @@ QVariant InterfaceTreeCacheModel::data(const QModelIndex &index, int role) const
          * are supported at the moment, so the information to be displayed is pretty limited.
          * After saving, the devices are stored in global_capture_opts and no longer
          * classify as new devices. */
-        interface_t * device = lookup(index);
+        const interface_t * device = lookup(index);
 
         if ( device != 0 )
         {
@@ -557,10 +552,10 @@ QModelIndex InterfaceTreeCacheModel::index(int row, int column, const QModelInde
     return sourceModel->index(row, column, parent);
 }
 
-void InterfaceTreeCacheModel::addDevice(interface_t * newDevice)
+void InterfaceTreeCacheModel::addDevice(const interface_t * newDevice)
 {
     emit beginInsertRows(QModelIndex(), rowCount(), rowCount());
-    newDevices << newDevice;
+    newDevices << *newDevice;
     emit endInsertRows();
 }
 
diff --git a/ui/qt/interface_tree_cache_model.h b/ui/qt/interface_tree_cache_model.h
index cee1c0bea0..b5bd1ba8e3 100644
--- a/ui/qt/interface_tree_cache_model.h
+++ b/ui/qt/interface_tree_cache_model.h
@@ -51,7 +51,7 @@ public:
     void reset(int row);
     void save();
 
-    void addDevice(interface_t * newDevice);
+    void addDevice(const interface_t * newDevice);
     void deleteDevice(const QModelIndex &index);
 #endif
 
@@ -59,7 +59,7 @@ private:
     InterfaceTreeModel * sourceModel;
 
 #ifdef HAVE_LIBPCAP
-    QList<interface_t *> newDevices;
+    QList<interface_t> newDevices;
 
     void saveNewDevices();
 #endif
@@ -68,7 +68,7 @@ private:
     QList<InterfaceTreeColumns> checkableColumns;
 
 #ifdef HAVE_LIBPCAP
-    interface_t * lookup(const QModelIndex &index) const;
+    const interface_t * lookup(const QModelIndex &index) const;
 #endif
 
     bool changeIsAllowed(InterfaceTreeColumns col) const;
diff --git a/ui/qt/manage_interfaces_dialog.cpp b/ui/qt/manage_interfaces_dialog.cpp
index 23986a974e..19b0d312d9 100644
--- a/ui/qt/manage_interfaces_dialog.cpp
+++ b/ui/qt/manage_interfaces_dialog.cpp
@@ -222,24 +222,25 @@ void ManageInterfacesDialog::on_buttonBox_accepted()
 
 void ManageInterfacesDialog::on_addPipe_clicked()
 {
-    interface_t * device = g_new0(interface_t, 1);
-
-    device->name = qstring_strdup(tr("New Pipe"));
-    device->display_name = g_strdup(device->name);
-    device->hidden       = FALSE;
-    device->selected     = TRUE;
-    device->pmode        = global_capture_opts.default_options.promisc_mode;
-    device->has_snaplen  = global_capture_opts.default_options.has_snaplen;
-    device->snaplen      = global_capture_opts.default_options.snaplen;
-    device->cfilter      = g_strdup(global_capture_opts.default_options.cfilter);
+    interface_t device;
+
+    memset(&device, 0, sizeof(device));
+    device.name = qstring_strdup(tr("New Pipe"));
+    device.display_name = g_strdup(device.name);
+    device.hidden       = FALSE;
+    device.selected     = TRUE;
+    device.pmode        = global_capture_opts.default_options.promisc_mode;
+    device.has_snaplen  = global_capture_opts.default_options.has_snaplen;
+    device.snaplen      = global_capture_opts.default_options.snaplen;
+    device.cfilter      = g_strdup(global_capture_opts.default_options.cfilter);
 #ifdef CAN_SET_CAPTURE_BUFFER_SIZE
-    device->buffer       = DEFAULT_CAPTURE_BUFFER_SIZE;
+    device.buffer       = DEFAULT_CAPTURE_BUFFER_SIZE;
 #endif
-    device->active_dlt = -1;
-    device->if_info.name = g_strdup(device->name);
-    device->if_info.type = IF_PIPE;
+    device.active_dlt = -1;
+    device.if_info.name = g_strdup(device.name);
+    device.if_info.type = IF_PIPE;
 
-    sourceModel->addDevice(device);
+    sourceModel->addDevice(&device);
     updateWidgets();
 }