From bfaef4cf48adc371d855e08fecbdd65a8344fd16 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Thu, 11 Apr 2019 15:08:23 -0700 Subject: [PATCH] Give a PacketDialog a Buffer and read directly into it and into its wtap_rec. We may or may not be working on the currently-selected packet, so there's no reason to use read into the capture_file's wtap_rec and Buffer for the currently-selected packet. We already have a wtap_rec of our own, and we currently have a pointer to a raw packet data array that we can replace with a Buffer of our own; just read into them. Use wtap_rec_init() on the wtap_rec, rather than using its implicit constructor - there's no guarantee that the initial values of the structure members, as defined by C (and C++), are what we want. Use wtap_rec_cleanup() in the destructor; it might do more than the implied destructor (which does nothing). wtap_rec and Buffer are C structures, so they don't get C++ constructors and destructors - we have to use the C ones, which are explicit functions. I think there are memory leaks that this fixes (packet comments and Buffer for the options data, leaked when a PacketDialog window is closed). Change-Id: Ica1d937fd00e4d2f5e4e2275bcd8edddb7a7921b Reviewed-on: https://code.wireshark.org/review/32832 Petri-Dish: Guy Harris Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- ui/qt/packet_dialog.cpp | 23 ++++++++++------------- ui/qt/packet_dialog.h | 5 +++-- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/ui/qt/packet_dialog.cpp b/ui/qt/packet_dialog.cpp index e35ec88885..ea917b1535 100644 --- a/ui/qt/packet_dialog.cpp +++ b/ui/qt/packet_dialog.cpp @@ -34,13 +34,15 @@ PacketDialog::PacketDialog(QWidget &parent, CaptureFile &cf, frame_data *fdata) WiresharkDialog(parent, cf), ui(new Ui::PacketDialog), proto_tree_(NULL), - byte_view_tab_(NULL), - rec_(wtap_rec()), - packet_data_(NULL) + byte_view_tab_(NULL) { ui->setupUi(this); loadGeometry(parent.width() * 4 / 5, parent.height() * 4 / 5); ui->hintLabel->setSmallText(); + + wtap_rec_init(&rec_); + ws_buffer_init(&buf_, 1514); + edt_.session = NULL; edt_.tvb = NULL; edt_.tree = NULL; @@ -49,23 +51,17 @@ PacketDialog::PacketDialog(QWidget &parent, CaptureFile &cf, frame_data *fdata) setWindowSubtitle(tr("Packet %1").arg(fdata->num)); - if (!cf_read_record(cap_file_.capFile(), fdata)) { + if (!cf_read_record_r(cap_file_.capFile(), fdata, &rec_, &buf_)) { reject(); return; } - rec_ = cap_file_.capFile()->rec; - -#ifndef __clang_analyzer__ - packet_data_ = (guint8 *) g_memdup(ws_buffer_start_ptr(&(cap_file_.capFile()->buf)), fdata->cap_len); -#endif - - /* proto tree, visible. We need a proto tree if there's custom columns */ + /* proto tree, visible. We need a proto tree if there are custom columns */ epan_dissect_init(&edt_, cap_file_.capFile()->epan, TRUE, TRUE); col_custom_prime_edt(&edt_, &(cap_file_.capFile()->cinfo)); epan_dissect_run(&edt_, cap_file_.capFile()->cd_t, &rec_, - frame_tvbuff_new(&cap_file_.capFile()->provider, fdata, packet_data_), + frame_tvbuff_new_buffer(&cap_file_.capFile()->provider, fdata, &buf_), fdata, &(cap_file_.capFile()->cinfo)); epan_dissect_fill_in_columns(&edt_, TRUE, TRUE); @@ -108,7 +104,8 @@ PacketDialog::~PacketDialog() { delete ui; epan_dissect_cleanup(&edt_); - g_free(packet_data_); + wtap_rec_cleanup(&rec_); + ws_buffer_free(&buf_); } void PacketDialog::captureFileClosing() diff --git a/ui/qt/packet_dialog.h b/ui/qt/packet_dialog.h index 7beb1ff92a..d4bf898d0f 100644 --- a/ui/qt/packet_dialog.h +++ b/ui/qt/packet_dialog.h @@ -14,6 +14,7 @@ #include "epan/epan_dissect.h" #include "wiretap/wtap.h" +#include "wsutil/buffer.h" #include @@ -44,9 +45,9 @@ private: QString col_info_; ProtoTree *proto_tree_; ByteViewTab *byte_view_tab_; - epan_dissect_t edt_; wtap_rec rec_; - guint8 *packet_data_; + Buffer buf_; + epan_dissect_t edt_; }; #endif // PACKET_DIALOG_H