osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

Qt: Remove unnecessary ColumnText object 

wmem should not be used inside a model, as the memory managment of
those models is part of the responsibility of Qt. It could happen,
that the file is closed, but the model still needs to access the
information, in which case the memory access fails.

Change-Id: I740a4bae61cc32f0f0245515c23abf175ef588f1
Reviewed-on: https://code.wireshark.org/review/33622
Petri-Dish: Roland Knall <rknall@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>
This commit is contained in:
Roland Knall 2019-06-16 13:36:51 +02:00
parent 64badc5a3a
commit d95262bf74
3 changed files with 31 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
ui/qt/models/packet_list_model.cpp
View File

@ -318,7 +318,7 @@ void PacketListModel::sort(int column, Qt::SortOrder order)
emit pushProgressStatus(tr("Dissecting"), true, true, &stop_flag);
int row_num = 0;
foreach (PacketListRecord *row, physical_rows_) {
row->columnString(sort_cap_file_, column);
row->ensureDissection(sort_cap_file_, column);
row_num++;
if (busy_timer_.elapsed() > busy_timeout_) {
if (stop_flag) {
@ -458,7 +458,7 @@ bool PacketListModel::recordLessThan(PacketListRecord *r1, PacketListRecord *r2)
// Column comes directly from frame data
cmp_val = frame_data_compare(sort_cap_file_->epan, r1->frameData(), r2->frameData(), sort_cap_file_->cinfo.columns[sort_column_].col_fmt);
} else {
if (r1->columnString(sort_cap_file_, sort_column_).constData() == r2->columnString(sort_cap_file_, sort_column_).constData()) {
if (r1->columnString(sort_cap_file_, sort_column_).compare(r2->columnString(sort_cap_file_, sort_column_)) == 0) {
cmp_val = 0;
} else if (sort_column_is_numeric_) {
// Custom column with numeric data (or something like a port number).
@ -478,7 +478,7 @@ bool PacketListModel::recordLessThan(PacketListRecord *r1, PacketListRecord *r2)
cmp_val = 1;
}
} else {
cmp_val = strcmp(r1->columnString(sort_cap_file_, sort_column_).constData(), r2->columnString(sort_cap_file_, sort_column_).constData());
cmp_val = r1->columnString(sort_cap_file_, sort_column_).compare(r2->columnString(sort_cap_file_, sort_column_));
}
if (cmp_val == 0) {
@ -595,7 +595,7 @@ QVariant PacketListModel::data(const QModelIndex &d_index, int role) const
case Qt::DisplayRole:
{
int column = d_index.column();
QByteArray column_string = record->columnString(cap_file_, column, true);
QString column_string = record->columnString(cap_file_, column, true);
// We don't know an item's sizeHint until we fetch its text here.
// Assume each line count is 1. If the line count changes, emit
// itemHeightChanged which triggers another redraw (including a
@ -741,7 +741,7 @@ void PacketListModel::ensureRowColorized(int row)
if (!record)
return;
if (!record->colorized()) {
record->columnString(cap_file_, 1, true);
record->ensureDissection(cap_file_, 1, true);
}
}

50
ui/qt/models/packet_list_record.cpp
View File

@ -23,21 +23,10 @@
#include <QStringList>
class ColumnTextList : public QList<const char *> {
public:
// Allocate our records using wmem.
static void *operator new(size_t size) {
return wmem_alloc(wmem_file_scope(), size);
}
static void operator delete(void *) {}
};
QMap<int, int> PacketListRecord::cinfo_column_;
unsigned PacketListRecord::col_data_ver_ = 1;
PacketListRecord::PacketListRecord(frame_data *frameData, struct _GStringChunk *string_cache_pool) :
col_text_(0),
fdata_(frameData),
lines_(1),
line_count_changed_(false),
@ -55,21 +44,30 @@ void *PacketListRecord::operator new(size_t size)
// We might want to return a const char * instead. This would keep us from
// creating excessive QByteArrays, e.g. in PacketListModel::recordLessThan.
const QByteArray PacketListRecord::columnString(capture_file *cap_file, int column, bool colorized)
const QString PacketListRecord::columnString(capture_file *cap_file, int column, bool colorized)
{
if ( ensureDissection(cap_file, column, colorized) )
return col_text_.at(column);
return QString();
}
bool PacketListRecord::ensureDissection(capture_file *cap_file, int column, bool colorized)
{
// packet_list_store.c:packet_list_get_value
g_assert(fdata_);
Q_ASSERT(fdata_);
if (!cap_file || column < 0 || column > cap_file->cinfo.num_cols) {
return QByteArray();
return false;
}
bool dissect_color = colorized && !colorized_;
if (!col_text_ || column >= col_text_->size() || !col_text_->at(column) || data_ver_ != col_data_ver_ || dissect_color) {
if (col_text_.isEmpty() || column >= col_text_.size() || data_ver_ != col_data_ver_ || dissect_color) {
dissect(cap_file, dissect_color);
}
return col_text_->value(column, QByteArray());
return true;
}
void PacketListRecord::resetColumns(column_info *cinfo)
@ -104,8 +102,7 @@ void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color)
wtap_rec rec; /* Record metadata */
Buffer buf; /* Record data */
if (!col_text_) col_text_ = new ColumnTextList;
gboolean dissect_columns = col_text_->isEmpty() || data_ver_ != col_data_ver_;
gboolean dissect_columns = col_text_.isEmpty() || data_ver_ != col_data_ver_;
if (!cap_file) {
return;
@ -206,11 +203,8 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
return;
}
if (col_text_) {
col_text_->clear();
} else {
col_text_ = new ColumnTextList;
}
col_text_.clear();
lines_ = 1;
line_count_changed_ = false;
@ -223,7 +217,7 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
/* Column based on frame_data or it already contains a value */
if (text_col < 0) {
col_fill_in_frame_data(fdata_, cinfo, column, FALSE);
col_text_->append(cinfo->columns[column].col_data);
col_text_.append(cinfo->columns[column].col_data);
continue;
}
@ -240,7 +234,7 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
// XXX - ui/gtk/packet_list_store.c uses G_MAXUSHORT. We don't do proper UTF8
// truncation in either case.
int col_text_len = MIN(qstrlen(cinfo->col_data[column]) + 1, COL_MAX_INFO_LEN);
col_text_->append(QByteArray::fromRawData(cinfo->columns[column].col_data, col_text_len));
col_text_.append(QByteArray::fromRawData(cinfo->columns[column].col_data, col_text_len));
break;
}
/* !! FALL-THROUGH!! */
@ -267,9 +261,9 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
if (!get_column_resolved(column) && cinfo->col_expr.col_expr_val[column]) {
/* Use the unresolved value in col_expr_val */
// XXX Use QContiguousCache?
col_text_->append(cinfo->col_expr.col_expr_val[column]);
col_text_.append(cinfo->col_expr.col_expr_val[column]);
} else {
col_text_->append(cinfo->columns[column].col_data);
col_text_.append(cinfo->columns[column].col_data);
}
break;
}
@ -291,7 +285,7 @@ void PacketListRecord::cacheColumnStrings(column_info *cinfo)
// https://git.gnome.org/browse/glib/tree/glib/gstringchunk.c
// We might be better off adding the equivalent functionality to
// wmem_tree.
col_text_->append(g_string_chunk_insert_const(string_cache_pool_, col_str));
col_text_.append(g_string_chunk_insert_const(string_cache_pool_, col_str));
for (int i = 0; col_str[i]; i++) {
if (col_str[i] == '\n') col_lines++;
}

7
ui/qt/models/packet_list_record.h
View File

@ -38,13 +38,15 @@ public:
static void operator delete(void *) {}
// Return the string value for a column. Data is cached if possible.
const QByteArray columnString(capture_file *cap_file, int column, bool colorized = false);
const QString columnString(capture_file *cap_file, int column, bool colorized = false);
frame_data *frameData() const { return fdata_; }
// packet_list->col_to_text in gtk/packet_list_store.c
static int textColumn(int column) { return cinfo_column_.value(column, -1); }
bool colorized() { return colorized_; }
struct conversation *conversation() { return conv_; }
bool ensureDissection(capture_file * cap_file, int column, bool colorized = false);
int columnTextSize(const char *str);
static void invalidateAllRecords() { col_data_ver_++; }
static void resetColumns(column_info *cinfo);
@ -53,8 +55,7 @@ public:
inline int lineCountChanged() { return line_count_changed_; }
private:
/** The column text for some columns */
ColumnTextList *col_text_;
QStringList col_text_;
frame_data *fdata_;
int lines_;