From 6bc0ba845100acb3c721e4f945bafeb1bed6c942 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Mon, 26 Jun 2017 23:40:22 +0200 Subject: 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 Tested-by: Petri Dish Buildbot Reviewed-by: Roland Knall --- ui/qt/interface_tree_cache_model.cpp | 27 +++++++++++---------------- ui/qt/interface_tree_cache_model.h | 6 +++--- ui/qt/manage_interfaces_dialog.cpp | 31 ++++++++++++++++--------------- 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::const_iterator it = newDevices.constBegin(); + QList::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(&(*it)); bool useDevice = false; QMap * 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 newDevices; + QList newDevices; void saveNewDevices(); #endif @@ -68,7 +68,7 @@ private: QList 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(); } -- cgit v1.2.1