Qt: fix memleaks on opening a context menu
authorPeter Wu <peter@lekensteyn.nl>
Sun, 27 May 2018 09:48:57 +0000 (11:48 +0200)
committerAnders Broman <a.broman58@gmail.com>
Wed, 30 May 2018 08:20:42 +0000 (08:20 +0000)
FrameInformation was never deallocated, leaking the whole pinfo scope.
Fix a dealloc-alloc-mismatch (packet_data_ was g_memdup'd). Attach the
DataPrinter menu actions to the action group instead of the singleton
DataPrinter instance, this enables freeing the actions when the submenu
is gone rather than clearing this at program exit.

Reported by ASAN.

Change-Id: If13af94a60b07b0e52973ccc5c437ef987bfb394
Fixes: v2.5.0rc0-1627-g8a6ea0e454 ("Qt: Further cleanup ByteView")
Reviewed-on: https://code.wireshark.org/review/27844
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
ui/qt/packet_list.cpp
ui/qt/utils/data_printer.cpp
ui/qt/utils/frame_information.cpp

index 13362fb4f67353048e076610a57fedcbb00e827e..5b275fb6ccb4d2cd0d7b3aa4898f45cccd8dd2dc 100644 (file)
@@ -503,6 +503,7 @@ void PacketList::contextMenuEvent(QContextMenuEvent *event)
     proto_prefs_menu_.setModule(module_name);
 
     QModelIndex ctxIndex = indexAt(event->pos());
+    // frameData will be owned by one of the submenus, see below.
     FrameInformation * frameData =
             new FrameInformation(new CaptureFile(this, cap_file_), packet_list_model_->getRowFdata(ctxIndex.row()));
 
@@ -589,6 +590,8 @@ void PacketList::contextMenuEvent(QContextMenuEvent *event)
 
     QActionGroup * copyEntries = DataPrinter::copyActions(this, frameData);
     submenu->addActions(copyEntries->actions());
+    copyEntries->setParent(submenu);
+    frameData->setParent(submenu);
 
     ctx_menu_.addSeparator();
     ctx_menu_.addMenu(&proto_prefs_menu_);
index 8888c0960f673d515907555bc17b1faa260d1ecd..cf2e2604cec2abafd80accaff0f9799fd64159a7 100644 (file)
@@ -191,41 +191,35 @@ QActionGroup * DataPrinter::copyActions(QObject * copyClass, QObject * data)
         actions->setProperty("idataprintable", VariantPointer<QObject>::asQVariant(copyClass));
 
     // Mostly duplicated from main_window.ui
-    QAction * action = new QAction(tr("Copy Bytes as Hex + ASCII Dump"), dpi);
+    QAction * action = new QAction(tr("Copy Bytes as Hex + ASCII Dump"), actions);
     action->setToolTip(tr("Copy packet bytes as a hex and ASCII dump."));
     action->setProperty("printertype", DataPrinter::DP_HexDump);
     connect(action, SIGNAL(triggered(bool)), dpi, SLOT(copyIDataBytes(bool)));
-    actions->addAction(action);
 
-    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Hex Dump"), dpi);
+    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Hex Dump"), actions);
     action->setToolTip(tr("Copy packet bytes as a hex dump."));
     action->setProperty("printertype", DataPrinter::DP_HexOnly);
     connect(action, SIGNAL(triggered(bool)), dpi, SLOT(copyIDataBytes(bool)));
-    actions->addAction(action);
 
-    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Printable Text"), dpi);
+    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Printable Text"), actions);
     action->setToolTip(tr("Copy only the printable text in the packet."));
     action->setProperty("printertype", DataPrinter::DP_PrintableText);
     connect(action, SIGNAL(triggered(bool)), dpi, SLOT(copyIDataBytes(bool)));
-    actions->addAction(action);
 
-    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as a Hex Stream"), dpi);
+    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as a Hex Stream"), actions);
     action->setToolTip(tr("Copy packet bytes as a stream of hex."));
     action->setProperty("printertype", DataPrinter::DP_HexStream);
     connect(action, SIGNAL(triggered(bool)), dpi, SLOT(copyIDataBytes(bool)));
-    actions->addAction(action);
 
-    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Raw Binary"), dpi);
+    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Raw Binary"), actions);
     action->setToolTip(tr("Copy packet bytes as application/octet-stream MIME data."));
     action->setProperty("printertype", DataPrinter::DP_Binary);
     connect(action, SIGNAL(triggered(bool)), dpi, SLOT(copyIDataBytes(bool)));
-    actions->addAction(action);
 
-    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Escaped String"), dpi);
+    action = new QAction(tr(UTF8_HORIZONTAL_ELLIPSIS "as Escaped String"), actions);
     action->setToolTip(tr("Copy packet bytes as an escaped string."));
     action->setProperty("printertype", DataPrinter::DP_EscapedString);
     connect(action, SIGNAL(triggered(bool)), dpi, SLOT(copyIDataBytes(bool)));
-    actions->addAction(action);
 
     return actions;
 }
index ec7d7c988c758159b7f100d34bbdb727dc064631..68acb67bdbb4b550e9813217a5752a8185b91297 100644 (file)
@@ -60,7 +60,7 @@ void FrameInformation::loadFrameTree()
 FrameInformation::~FrameInformation()
 {
     epan_dissect_cleanup(&edt_);
-    delete(packet_data_);
+    g_free(packet_data_);
 }
 
 bool FrameInformation::isValid()