From 4cdd17a59dca0c9d2597d7974846e2b8ad9e1462 Mon Sep 17 00:00:00 2001 From: Gerald Combs Date: Sat, 31 May 2014 09:56:35 +0800 Subject: [PATCH] Qt: Cache packet list column strings. For each displayed packet list row, save a copy of or a pointer to column strings similar to ui/gtk/packet_list_store.c. This lets us call epan_dissect_run only once per row. Bug: 9511 Change-Id: I17e8ebeb5ed70518c9047413c3b2a46f01e904ef Reviewed-on: https://code.wireshark.org/review/2752 Reviewed-by: Anders Broman --- epan/CMakeLists.txt | 1 + ui/qt/packet_list_model.cpp | 120 +++-------------- ui/qt/packet_list_record.cpp | 216 ++++++++++++++++++++++++++++-- ui/qt/packet_list_record.h | 21 ++- ui/qt/related_packet_delegate.cpp | 2 +- 5 files changed, 238 insertions(+), 122 deletions(-) diff --git a/epan/CMakeLists.txt b/epan/CMakeLists.txt index 017b784997..ea3fcced7c 100644 --- a/epan/CMakeLists.txt +++ b/epan/CMakeLists.txt @@ -1673,6 +1673,7 @@ set_target_properties(epan PROPERTIES VERSION ${FULL_SO_VERSION} SOVERSION 0) ABICHECK(libwireshark) +file(GLOB COLUMN_INFO_HEADER column-info.h) file(GLOB CRYPT_HEADERS crypt/*.h) file(GLOB DFILTER_HEADERS dfilter/*.h ../tools/lemon/cppmagic.h) file(GLOB D_HEADERS dissectors/*.h) diff --git a/ui/qt/packet_list_model.cpp b/ui/qt/packet_list_model.cpp index 098f784648..db9cf0d243 100644 --- a/ui/qt/packet_list_model.cpp +++ b/ui/qt/packet_list_model.cpp @@ -21,9 +21,6 @@ #include "packet_list_model.h" -#include -#include -#include #include #include @@ -41,12 +38,13 @@ PacketListModel::PacketListModel(QObject *parent, capture_file *cf) : QAbstractItemModel(parent) { - cap_file_ = cf; + setCaptureFile(cf); } void PacketListModel::setCaptureFile(capture_file *cf) { cap_file_ = cf; + resetColumns(); } // Packet list records have no children (for now, at least). @@ -86,9 +84,9 @@ guint PacketListModel::recreateVisibleRows() endResetModel(); beginInsertRows(QModelIndex(), pos, pos); foreach (record, physical_rows_) { - if (record->getFdata()->flags.passed_dfilter || record->getFdata()->flags.ref_time) { + if (record->frameData()->flags.passed_dfilter || record->frameData()->flags.ref_time) { visible_rows_ << record; - number_to_row_[record->getFdata()->num] = visible_rows_.count() - 1; + number_to_row_[record->frameData()->num] = visible_rows_.count() - 1; } } endInsertRows(); @@ -110,6 +108,9 @@ void PacketListModel::clear() { void PacketListModel::resetColumns() { beginResetModel(); + if (cap_file_) { + PacketListRecord::resetColumns(&cap_file_->cinfo); + } endResetModel(); } @@ -136,7 +137,7 @@ QVariant PacketListModel::data(const QModelIndex &index, int role) const PacketListRecord *record = static_cast(index.internalPointer()); if (!record) return QVariant(); - frame_data *fdata = record->getFdata(); + frame_data *fdata = record->frameData(); if (!fdata) return QVariant(); @@ -190,106 +191,15 @@ QVariant PacketListModel::data(const QModelIndex &index, int role) const } return QColor(color->red >> 8, color->green >> 8, color->blue >> 8); case Qt::DisplayRole: - // Need packet data -- fall through - break; + { + int column = index.column(); + // g_log(NULL, G_LOG_LEVEL_DEBUG, "showing col %d", col_num); + return record->columnString(cap_file_, column); + } default: return QVariant(); } - int col_num = index.column(); -// g_log(NULL, G_LOG_LEVEL_DEBUG, "showing col %d", col_num); - - if (col_num > prefs.num_cols) - return QVariant(); - - epan_dissect_t edt; - column_info *cinfo; - gboolean create_proto_tree; - struct wtap_pkthdr phdr; /* Packet header */ - Buffer buf; /* Packet data */ - gboolean dissect_columns = TRUE; // XXX - Currently only a placeholder - - if (dissect_columns && cap_file_) - cinfo = &cap_file_->cinfo; - else - cinfo = NULL; - - memset(&phdr, 0, sizeof(struct wtap_pkthdr)); - - buffer_init(&buf, 1500); - if (!cap_file_ || !cf_read_record_r(cap_file_, fdata, &phdr, &buf)) { - /* - * Error reading the record. - * - * Don't set the color filter for now (we might want - * to colorize it in some fashion to warn that the - * row couldn't be filled in or colorized), and - * set the columns to placeholder values, except - * for the Info column, where we'll put in an - * error message. - */ - if (dissect_columns) { - col_fill_in_error(cinfo, fdata, FALSE, FALSE /* fill_fd_columns */); - - // for(gint col = 0; col < cinfo->num_cols; ++col) { - // /* Skip columns based on frame_data because we already store those. */ - // if (!col_based_on_frame_data(cinfo, col)) - // packet_list_change_record(packet_list, record->physical_pos, col, cinfo); - // } - // record->columnized = TRUE; - } - if (enable_color_) { - fdata->color_filter = NULL; - // record->colorized = TRUE; - } - buffer_free(&buf); - return QVariant(); /* error reading the record */ - } - - create_proto_tree = (color_filters_used() && enable_color_) || - (have_custom_cols(cinfo) && dissect_columns); - - epan_dissect_init(&edt, cap_file_->epan, - create_proto_tree, - FALSE /* proto_tree_visible */); - - if (enable_color_) - color_filters_prime_edt(&edt); - if (dissect_columns) - col_custom_prime_edt(&edt, cinfo); - - epan_dissect_run(&edt, cap_file_->cd_t, &phdr, frame_tvbuff_new_buffer(fdata, &buf), fdata, cinfo); - - if (enable_color_) - fdata->color_filter = color_filters_colorize_packet(&edt); - - if (dissect_columns) { - /* "Stringify" non frame_data vals */ - epan_dissect_fill_in_columns(&edt, FALSE, FALSE /* fill_fd_columns */); - - // for(col = 0; col < cinfo->num_cols; ++col) { - // /* Skip columns based on frame_data because we already store those. */ - // if (!col_based_on_frame_data(cinfo, col)) - // packet_list_change_record(packet_list, record->physical_pos, col, cinfo); - // } -// g_log(NULL, G_LOG_LEVEL_DEBUG, "d_c %d: %s", col_num, cinfo->col_data[col_num]); - } - - // if (dissect_columns) - // record->columnized = TRUE; - // if (enable_color_) - // record->colorized = TRUE; - - epan_dissect_cleanup(&edt); - buffer_free(&buf); - - switch (role) { - case Qt::DisplayRole: - return record->data(col_num, cinfo); - break; - default: - break; - } return QVariant(); } @@ -334,14 +244,14 @@ frame_data *PacketListModel::getRowFdata(int row) { PacketListRecord *record = visible_rows_[row]; if (!record) return NULL; - return record->getFdata(); + return record->frameData(); } int PacketListModel::visibleIndexOf(frame_data *fdata) const { int row = 0; foreach (PacketListRecord *record, visible_rows_) { - if (record->getFdata() == fdata) { + if (record->frameData() == fdata) { return row; } row++; diff --git a/ui/qt/packet_list_record.cpp b/ui/qt/packet_list_record.cpp index 0090cf0da3..2bad6f8032 100644 --- a/ui/qt/packet_list_record.cpp +++ b/ui/qt/packet_list_record.cpp @@ -21,30 +21,228 @@ #include "packet_list_record.h" +#include + +#include +#include +#include + +#include "color.h" +#include "color_filters.h" +#include "frame_tvbuff.h" + +#include + +QMap cinfo_column_; + PacketListRecord::PacketListRecord(frame_data *frameData) : - //col_text_(NULL), - //col_text_len_(NULL), - fdata_(frameData) + fdata_(frameData), +// columnized_(false), + colorized_(false) { } -QVariant PacketListRecord::data(int col_num, column_info *cinfo) const +QVariant PacketListRecord::columnString(capture_file *cap_file, int column) { + // packet_list_store.c:packet_list_get_value g_assert(fdata_); - if (!cinfo) + if (!cap_file || column < 0 || column > cap_file->cinfo.num_cols) { return QVariant(); + } - if (col_based_on_frame_data(cinfo, col_num)) - col_fill_in_frame_data(fdata_, cinfo, col_num, FALSE); + if (column >= col_text_.size() || col_text_[column].isNull() || !colorized_) { + dissect(cap_file, !colorized_); + } - return cinfo->col_data[col_num]; + return col_text_.value(column, QByteArray()); } -frame_data *PacketListRecord::getFdata() { +frame_data *PacketListRecord::frameData() { return fdata_; } +void PacketListRecord::resetColumns(column_info *cinfo) +{ + if (!cinfo) { + return; + } + + cinfo_column_.clear(); + int i, j; + for (i = 0, j = 0; i < cinfo->num_cols; i++) { + if (!col_based_on_frame_data(cinfo, i)) { + cinfo_column_[i] = j; + j++; + } + } +} + +void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color) +{ + // packet_list_store.c:packet_list_dissect_and_cache_record + epan_dissect_t edt; + column_info *cinfo = NULL; + gboolean create_proto_tree; + struct wtap_pkthdr phdr; /* Packet header */ + Buffer buf; /* Packet data */ + gboolean dissect_columns = col_text_.isEmpty(); + + if (!cap_file) { + return; + } + + memset(&phdr, 0, sizeof(struct wtap_pkthdr)); + + if (dissect_columns) { + cinfo = &cap_file->cinfo; + } + + buffer_init(&buf, 1500); + if (!cf_read_record_r(cap_file, fdata_, &phdr, &buf)) { + /* + * Error reading the record. + * + * Don't set the color filter for now (we might want + * to colorize it in some fashion to warn that the + * row couldn't be filled in or colorized), and + * set the columns to placeholder values, except + * for the Info column, where we'll put in an + * error message. + */ + if (dissect_columns) { + col_fill_in_error(cinfo, fdata_, FALSE, FALSE /* fill_fd_columns */); + + cacheColumnStrings(cinfo); + } + if (dissect_color) { + fdata_->color_filter = NULL; + colorized_ = TRUE; + } + buffer_free(&buf); + return; /* error reading the record */ + } + + create_proto_tree = (dissect_color && color_filters_used()) || + (dissect_columns && have_custom_cols(cinfo)); + + epan_dissect_init(&edt, cap_file->epan, + create_proto_tree, + FALSE /* proto_tree_visible */); + + if (dissect_color) + color_filters_prime_edt(&edt); + if (dissect_columns) + col_custom_prime_edt(&edt, cinfo); + + /* + * XXX - need to catch an OutOfMemoryError exception and + * attempt to recover from it. + */ + epan_dissect_run(&edt, cap_file->cd_t, &phdr, frame_tvbuff_new_buffer(fdata_, &buf), fdata_, cinfo); + + if (dissect_color) + fdata_->color_filter = color_filters_colorize_packet(&edt); + + if (dissect_columns) { + /* "Stringify" non frame_data vals */ + epan_dissect_fill_in_columns(&edt, FALSE, FALSE /* fill_fd_columns */); + cacheColumnStrings(cinfo); + } + + if (dissect_color) { + colorized_ = true; + } + + epan_dissect_cleanup(&edt); + buffer_free(&buf); +} + +//#define MINIMIZE_STRING_COPYING 1 +void PacketListRecord::cacheColumnStrings(column_info *cinfo) +{ + // packet_list_store.c:packet_list_change_record(PacketList *packet_list, PacketListRecord *record, gint col, column_info *cinfo) + if (!cinfo) { + return; + } + + col_text_.clear(); + + for (int column = 0; column < cinfo->num_cols; ++column) { + +#ifdef MINIMIZE_STRING_COPYING + int text_col = cinfo_column_.value(column, -1); + + /* 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->col_data[column]); + continue; + } + + switch (cinfo->col_fmt[column]) { + case COL_DEF_SRC: + case COL_RES_SRC: /* COL_DEF_SRC is currently just like COL_RES_SRC */ + case COL_UNRES_SRC: + case COL_DEF_DL_SRC: + case COL_RES_DL_SRC: + case COL_UNRES_DL_SRC: + case COL_DEF_NET_SRC: + case COL_RES_NET_SRC: + case COL_UNRES_NET_SRC: + case COL_DEF_DST: + case COL_RES_DST: /* COL_DEF_DST is currently just like COL_RES_DST */ + case COL_UNRES_DST: + case COL_DEF_DL_DST: + case COL_RES_DL_DST: + case COL_UNRES_DL_DST: + case COL_DEF_NET_DST: + case COL_RES_NET_DST: + case COL_UNRES_NET_DST: + case COL_PROTOCOL: + case COL_INFO: + case COL_IF_DIR: + case COL_DCE_CALL: + case COL_8021Q_VLAN_ID: + case COL_EXPERT: + case COL_FREQ_CHAN: + if (cinfo->col_data[column] && cinfo->col_data[column] != cinfo->col_buf[column]) { + /* This is a constant string, so we don't have to copy it */ + // 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->col_data[column], col_text_len)); + break; + } + /* !! FALL-THROUGH!! */ + + default: + 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]); + } else { + col_text_.append(cinfo->col_data[column]); + } + break; + } +#else // MINIMIZE_STRING_COPYING + // XXX Use QContiguousCache? + if (!get_column_resolved(column) && cinfo->col_expr.col_expr_val[column]) { + /* Use the unresolved value in col_expr_val */ + col_text_.append(cinfo->col_expr.col_expr_val[column]); + } else { + int text_col = cinfo_column_.value(column, -1); + + if (text_col < 0) { + col_fill_in_frame_data(fdata_, cinfo, column, FALSE); + } + col_text_.append(cinfo->col_data[column]); + } +#endif // MINIMIZE_STRING_COPYING + } +} + /* * Editor modelines * diff --git a/ui/qt/packet_list_record.h b/ui/qt/packet_list_record.h index ebc2b3c4ba..574da0d912 100644 --- a/ui/qt/packet_list_record.h +++ b/ui/qt/packet_list_record.h @@ -26,9 +26,12 @@ #include +#include "cfile.h" + #include #include +#include #include #include @@ -36,21 +39,22 @@ class PacketListRecord { public: PacketListRecord(frame_data *frameData); - QVariant data(int col_num, column_info *cinfo) const; - frame_data *getFdata(); + // Return the string value for a column. Data is cached if possible. + QVariant columnString(capture_file *cap_file, int column); + frame_data *frameData(); + + static void resetColumns(column_info *cinfo); private: /** The column text for some columns */ - //gchar **col_text_; - /**< The length of the column text strings in 'col_text' */ - //guint *col_text_len_; + QList col_text_; frame_data *fdata_; /** Has this record been columnized? */ - //gboolean columnized_; +// gboolean columnized_; /** Has this record been colorized? */ - //gboolean colorized_; + bool colorized_; /* admin stuff used by the custom list model */ /** position within the physical array */ @@ -58,6 +62,9 @@ private: /** position within the visible array */ //gint visible_pos_; + void dissect(capture_file *cap_file, bool dissect_color = false); + void cacheColumnStrings(column_info *cinfo); + }; #endif // PACKET_LIST_RECORD_H diff --git a/ui/qt/related_packet_delegate.cpp b/ui/qt/related_packet_delegate.cpp index c68c9b46c7..11fb34caac 100644 --- a/ui/qt/related_packet_delegate.cpp +++ b/ui/qt/related_packet_delegate.cpp @@ -40,7 +40,7 @@ void RelatedPacketDelegate::paint(QPainter *painter, const QStyleOptionViewItem frame_data *fd; PacketListRecord *record = static_cast(index.internalPointer()); - if (!record || (fd = record->getFdata()) == NULL) { + if (!record || (fd = record->frameData()) == NULL) { return; }