diff --git a/file.c b/file.c index 71cc5905f2..499472b85c 100644 --- a/file.c +++ b/file.c @@ -4289,9 +4289,11 @@ cf_set_modified_block(capture_file *cf, frame_data *fd, const wtap_block_t new_b if (pkt_block == new_block) { /* No need to save anything here, the caller changes went right * onto the block. - * Unfortunately we don't have a way to know how many comments were in the block - * before the caller modified it. + * Unfortunately we don't have a way to know how many comments were + * in the block before the caller modified it, so tell the caller + * it is its responsibility to update the comment count. */ + return FALSE; } else { if (pkt_block) diff --git a/file.h b/file.h index 53a4369d38..0b7d8c905e 100644 --- a/file.h +++ b/file.h @@ -702,6 +702,10 @@ wtap_block_t cf_get_packet_block(capture_file *cf, const frame_data *fd); * @param cf the capture file * @param fd the frame_data structure for the frame * @param new_block the block replacing the old block + * + * @return TRUE if the block is modified for the first time. FALSE if + * the block was already modified before, in which case the caller is + * responsible for updating the comment count. */ gboolean cf_set_modified_block(capture_file *cf, frame_data *fd, const wtap_block_t new_block); diff --git a/ui/qt/models/packet_list_model.cpp b/ui/qt/models/packet_list_model.cpp index aa865ee24c..483d9d3ccc 100644 --- a/ui/qt/models/packet_list_model.cpp +++ b/ui/qt/models/packet_list_model.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include "ui/packet_list_utils.h" @@ -333,6 +334,145 @@ void PacketListModel::unsetAllFrameRefTime() emit dataChanged(index(0, 0), index(rowCount() - 1, columnCount() - 1)); } +void PacketListModel::addFrameComment(const QModelIndexList &indices, const QByteArray &comment) +{ + int sectionMax = columnCount() - 1; + frame_data *fdata; + if (!cap_file_) return; + + foreach (const auto &index, std::as_const(indices)) { + if (!index.isValid()) continue; + + PacketListRecord *record = static_cast(index.internalPointer()); + if (!record) continue; + + fdata = record->frameData(); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + wtap_block_add_string_option(pkt_block, OPT_COMMENT, comment.data(), comment.size()); + + if (!cf_set_modified_block(cap_file_, fdata, pkt_block)) { + cap_file_->packet_comment_count++; + expert_update_comment_count(cap_file_->packet_comment_count); + } + + // In case there are coloring rules or columns related to comments. + // (#12519) + // + // XXX: "Does any active coloring rule relate to frame data" + // could be an optimization. For columns, note that + // "col_based_on_frame_data" only applies to built in columns, + // not custom columns based on frame data. (Should we prevent + // custom columns based on frame data from being created, + // substituting them with the other columns?) + // + // Note that there are not currently any fields that depend on + // whether other frames have comments, unlike with time references + // and time shifts ("frame.time_relative", "frame.offset_shift", etc.) + // If there were, then we'd need to reset data for all frames instead + // of just the frames changed. + record->invalidateColorized(); + record->invalidateRecord(); + emit dataChanged(index.sibling(index.row(), 0), index.sibling(index.row(), sectionMax), + QVector() << Qt::BackgroundRole << Qt::ForegroundRole << Qt::DisplayRole); + } +} + +void PacketListModel::setFrameComment(const QModelIndex &index, const QByteArray &comment, guint c_number) +{ + int sectionMax = columnCount() - 1; + frame_data *fdata; + if (!cap_file_) return; + + if (!index.isValid()) return; + + PacketListRecord *record = static_cast(index.internalPointer()); + if (!record) return; + + fdata = record->frameData(); + + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + if (comment.isEmpty()) { + wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, c_number); + if (!cf_set_modified_block(cap_file_, fdata, pkt_block)) { + cap_file_->packet_comment_count--; + expert_update_comment_count(cap_file_->packet_comment_count); + } + } else { + wtap_block_set_nth_string_option_value(pkt_block, OPT_COMMENT, c_number, comment.data(), comment.size()); + cf_set_modified_block(cap_file_, fdata, pkt_block); + } + + record->invalidateColorized(); + record->invalidateRecord(); + emit dataChanged(index.sibling(index.row(), 0), index.sibling(index.row(), sectionMax), + QVector() << Qt::BackgroundRole << Qt::ForegroundRole << Qt::DisplayRole); +} + +void PacketListModel::deleteFrameComments(const QModelIndexList &indices) +{ + int sectionMax = columnCount() - 1; + frame_data *fdata; + if (!cap_file_) return; + + foreach (const auto &index, std::as_const(indices)) { + if (!index.isValid()) continue; + + PacketListRecord *record = static_cast(index.internalPointer()); + if (!record) continue; + + fdata = record->frameData(); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); + + if (n_comments) { + for (guint i = 0; i < n_comments; i++) { + wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, 0); + } + if (!cf_set_modified_block(cap_file_, fdata, pkt_block)) { + cap_file_->packet_comment_count -= n_comments; + expert_update_comment_count(cap_file_->packet_comment_count); + } + + record->invalidateColorized(); + record->invalidateRecord(); + emit dataChanged(index.sibling(index.row(), 0), index.sibling(index.row(), sectionMax), + QVector() << Qt::BackgroundRole << Qt::ForegroundRole << Qt::DisplayRole); + } + } +} + +void PacketListModel::deleteAllFrameComments() +{ + int row; + int sectionMax = columnCount() - 1; + if (!cap_file_) return; + + /* XXX: we might need a progressbar here */ + + foreach (PacketListRecord *record, physical_rows_) { + frame_data *fdata = record->frameData(); + wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); + guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); + + if (n_comments) { + for (guint i = 0; i < n_comments; i++) { + wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, 0); + } + cf_set_modified_block(cap_file_, fdata, pkt_block); + + record->invalidateColorized(); + record->invalidateRecord(); + row = packetNumberToRow(fdata->num); + if (row > -1) { + emit dataChanged(index(row, 0), index(row, sectionMax), + QVector() << Qt::BackgroundRole << Qt::ForegroundRole << Qt::DisplayRole); + } + } + } + cap_file_->packet_comment_count = 0; + expert_update_comment_count(cap_file_->packet_comment_count); +} + void PacketListModel::setMaximumRowHeight(int height) { max_row_height_ = height; diff --git a/ui/qt/models/packet_list_model.h b/ui/qt/models/packet_list_model.h index 6f44dd5752..11b42f5d72 100644 --- a/ui/qt/models/packet_list_model.h +++ b/ui/qt/models/packet_list_model.h @@ -74,6 +74,10 @@ public: void setDisplayedFrameIgnore(gboolean set); void toggleFrameRefTime(const QModelIndex &rt_index); void unsetAllFrameRefTime(); + void addFrameComment(const QModelIndexList &indices, const QByteArray &comment); + void setFrameComment(const QModelIndex &index, const QByteArray &comment, guint c_number); + void deleteFrameComments(const QModelIndexList &indices); + void deleteAllFrameComments(); void setMaximumRowHeight(int height); diff --git a/ui/qt/models/packet_list_record.h b/ui/qt/models/packet_list_record.h index 20399f3d87..47aa5621d1 100644 --- a/ui/qt/models/packet_list_record.h +++ b/ui/qt/models/packet_list_record.h @@ -44,6 +44,9 @@ public: unsigned int conversation() { return conv_index_; } int columnTextSize(const char *str); + + void invalidateColorized() { colorized_ = false; } + void invalidateRecord() { col_text_cache_.remove(fdata_->num); } static void invalidateAllRecords() { col_text_cache_.clear(); } /* In Qt 6, QCache maxCost is a qsizetype, but the QAbstractItemModel * number of rows is still an int, so we're limited to INT_MAX anyway. diff --git a/ui/qt/packet_list.cpp b/ui/qt/packet_list.cpp index 87988f7a39..b56b29c180 100644 --- a/ui/qt/packet_list.cpp +++ b/ui/qt/packet_list.cpp @@ -1421,78 +1421,52 @@ QString PacketList::getPacketComment(guint c_number) void PacketList::addPacketComment(QString new_comment) { - frame_data *fdata; - if (!cap_file_ || !packet_list_model_) return; if (new_comment.isEmpty()) return; QByteArray ba = new_comment.toUtf8(); - for (int i = 0; i < selectedRows().size(); i++) { - int row = selectedRows().at(i); - - fdata = packet_list_model_->getRowFdata(row); - - if (!fdata) continue; - - wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); - - /* - * Make sure this would fit in a pcapng option. - * - * XXX - 65535 is the maximum size for an option in pcapng; - * what if another capture file format supports larger - * comments? - */ - if (ba.size() > 65535) { - simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, - "That comment is too large to save in a capture file."); - return; - } - wtap_block_add_string_option(pkt_block, OPT_COMMENT, ba.data(), ba.size()); - - cf_set_modified_block(cap_file_, fdata, pkt_block); + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (ba.size() > 65535) { + simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, + "That comment is too large to save in a capture file."); + return; } - redrawVisiblePackets(); + if (selectionModel() && selectionModel()->hasSelection()) { + packet_list_model_->addFrameComment(selectionModel()->selectedRows(), ba); + drawCurrentPacket(); + } } void PacketList::setPacketComment(guint c_number, QString new_comment) { - int row = currentIndex().row(); - frame_data *fdata; + QModelIndex curIndex = currentIndex(); if (!cap_file_ || !packet_list_model_) return; - fdata = packet_list_model_->getRowFdata(row); - - if (!fdata) return; - - wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); - - /* Check if we are clearing the comment */ - if (new_comment.isEmpty()) { - wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, c_number); - } else { - QByteArray ba = new_comment.toUtf8(); - /* - * Make sure this would fit in a pcapng option. - * - * XXX - 65535 is the maximum size for an option in pcapng; - * what if another capture file format supports larger - * comments? - */ - if (ba.size() > 65535) { - simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, - "That comment is too large to save in a capture file."); - return; - } - wtap_block_set_nth_string_option_value(pkt_block, OPT_COMMENT, c_number, ba.data(), ba.size()); + QByteArray ba = new_comment.toUtf8(); + /* + * Make sure this would fit in a pcapng option. + * + * XXX - 65535 is the maximum size for an option in pcapng; + * what if another capture file format supports larger + * comments? + */ + if (ba.size() > 65535) { + simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, + "That comment is too large to save in a capture file."); + return; } - cf_set_modified_block(cap_file_, fdata, pkt_block); - - redrawVisiblePackets(); + packet_list_model_->setFrameComment(curIndex, ba, c_number); + drawCurrentPacket(); } QString PacketList::allPacketComments() @@ -1528,54 +1502,20 @@ QString PacketList::allPacketComments() void PacketList::deleteCommentsFromPackets() { - frame_data *fdata; - if (!cap_file_ || !packet_list_model_) return; - for (int i = 0; i < selectedRows().size(); i++) { - int row = selectedRows().at(i); - - fdata = packet_list_model_->getRowFdata(row); - - if (!fdata) continue; - - wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); - guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); - - for (guint j = 0; j < n_comments; j++) { - wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, 0); - } - - cf_set_modified_block(cap_file_, fdata, pkt_block); + if (selectionModel() && selectionModel()->hasSelection()) { + packet_list_model_->deleteFrameComments(selectionModel()->selectedRows()); + drawCurrentPacket(); } - - redrawVisiblePackets(); } void PacketList::deleteAllPacketComments() { - guint32 framenum; - frame_data *fdata; - QString buf_str; - guint i; + if (!cap_file_ || !packet_list_model_) return; - if (!cap_file_) - return; - - for (framenum = 1; framenum <= cap_file_->count ; framenum++) { - fdata = frame_data_sequence_find(cap_file_->provider.frames, framenum); - wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata); - guint n_comments = wtap_block_count_option(pkt_block, OPT_COMMENT); - - for (i = 0; i < n_comments; i++) { - wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, 0); - } - cf_set_modified_block(cap_file_, fdata, pkt_block); - } - - cap_file_->packet_comment_count = 0; - expert_update_comment_count(cap_file_->packet_comment_count); - redrawVisiblePackets(); + packet_list_model_->deleteAllFrameComments(); + drawCurrentPacket(); }