Fix bug #5189: Wireshark crashes when cancelling a large sort operation.

Essentially: Don't try to sort if the pre-requisite "columnization" step is stopped
             via the progressbar dialog window before the step completes.
Also: Fix a (very) minor bug wherein the sort-indicator not always cleared on the
      "previous column" when "No Sorting" was selected from a column context menu.
Also: Do minor code, comments & whitespace cleanup.

svn path=/trunk/; revision=36180
This commit is contained in:
Bill Meier 2011-03-12 19:02:24 +00:00
parent d3cff4643b
commit a37493fe16
3 changed files with 125 additions and 86 deletions

View File

@ -356,7 +356,7 @@ col_details_edit_dlg (gint col_id, GtkTreeViewColumn *col)
gtk_misc_set_alignment(GTK_MISC(label), 1.0f, 0.5f);
gtk_tooltips_set_tip (tooltips, label,
"Select which packet information to present in the column.", NULL);
format_cmb = gtk_combo_box_new_text();
for (i = 0; i < NUM_COL_FMTS; i++) {
gtk_combo_box_append_text(GTK_COMBO_BOX(format_cmb), col_format_desc(i));
@ -412,7 +412,7 @@ col_details_edit_dlg (gint col_id, GtkTreeViewColumn *col)
g_signal_connect(ok_bt, "clicked", G_CALLBACK(col_title_change_ok), win);
cur_fmt = get_column_format (col_id);
gtk_combo_box_set_active(GTK_COMBO_BOX(format_cmb), cur_fmt);
gtk_combo_box_set_active(GTK_COMBO_BOX(format_cmb), cur_fmt);
if (cur_fmt == COL_CUSTOM) {
gtk_entry_set_text(GTK_ENTRY(field_te), get_column_custom_field(col_id));
g_snprintf(custom_occurrence_str, sizeof(custom_occurrence_str), "%d", get_column_custom_occurrence(col_id));
@ -431,19 +431,37 @@ col_details_edit_dlg (gint col_id, GtkTreeViewColumn *col)
gtk_widget_show_all(win);
}
/* Process sort request;
* order: GTK_SORT_ASCENDING or GTK_SORT_DESCENDING
* sort_indicator: TRUE: set sort_indicator on column; FALSE: don't set ....
*
* If necessary, columns are first "columnized" for all rows in the packet-list; If this
* is not completed (i.e., stopped), then the sort request is aborted.
*/
static void
new_packet_list_sort_column (gint col_id, GtkTreeViewColumn *col, GtkSortType order)
new_packet_list_sort_column (gint col_id, GtkTreeViewColumn *col, GtkSortType order, gboolean sort_indicator)
{
GtkTreeViewColumn *prev_col = (GtkTreeViewColumn *)
GtkTreeViewColumn *prev_col;
if (col == NULL) {
col = gtk_tree_view_get_column(GTK_TREE_VIEW(packetlist->view), col_id);
}
g_assert(col);
if (!packet_list_do_packet_list_dissect_and_cache_all(packetlist, col_id)) {
return; /* "stopped": do not try to sort */
}
prev_col = (GtkTreeViewColumn *)
g_object_get_data(G_OBJECT(packetlist->view), E_MPACKET_LIST_PREV_COLUMN_KEY);
if (prev_col) {
gtk_tree_view_column_set_sort_indicator(prev_col, FALSE);
}
gtk_tree_view_column_set_sort_indicator(col, TRUE);
gtk_tree_view_column_set_sort_indicator(col, sort_indicator);
gtk_tree_view_column_set_sort_order (col, order);
g_object_set_data(G_OBJECT(packetlist->view), E_MPACKET_LIST_PREV_COLUMN_KEY, col);
gtk_tree_sortable_set_sort_column_id(GTK_TREE_SORTABLE(packetlist), col_id, order);
gtk_tree_sortable_set_sort_column_id(GTK_TREE_SORTABLE(packetlist), col_id, order); /* triggers sort callback */
scroll_to_current ();
}
@ -462,13 +480,11 @@ new_packet_list_column_clicked_cb (GtkTreeViewColumn *col, gpointer user_data _U
return;
if (!gtk_tree_view_column_get_sort_indicator(col)) {
new_packet_list_sort_column (col_id, col, GTK_SORT_ASCENDING);
new_packet_list_sort_column (col_id, col, GTK_SORT_ASCENDING, TRUE);
} else if (order == GTK_SORT_ASCENDING) {
new_packet_list_sort_column (col_id, col, GTK_SORT_DESCENDING);
new_packet_list_sort_column (col_id, col, GTK_SORT_DESCENDING, TRUE);
} else {
gtk_tree_view_column_set_sort_indicator(col, FALSE);
gtk_tree_sortable_set_sort_column_id(GTK_TREE_SORTABLE(packetlist), 0, GTK_SORT_ASCENDING);
scroll_to_current ();
new_packet_list_sort_column (0, NULL, GTK_SORT_ASCENDING, FALSE);
}
}
@ -610,15 +626,13 @@ new_packet_list_column_menu_cb (GtkWidget *w, gpointer user_data _U_, COLUMN_SEL
switch (action) {
case COLUMN_SELECTED_SORT_ASCENDING:
new_packet_list_sort_column (col_id, col, GTK_SORT_ASCENDING);
new_packet_list_sort_column (col_id, col, GTK_SORT_ASCENDING, TRUE);
break;
case COLUMN_SELECTED_SORT_DESCENDING:
new_packet_list_sort_column (col_id, col, GTK_SORT_DESCENDING);
new_packet_list_sort_column (col_id, col, GTK_SORT_DESCENDING, TRUE);
break;
case COLUMN_SELECTED_SORT_NONE:
gtk_tree_view_column_set_sort_indicator(col, FALSE);
gtk_tree_sortable_set_sort_column_id(GTK_TREE_SORTABLE(packetlist), 0, GTK_SORT_ASCENDING);
scroll_to_current ();
new_packet_list_sort_column (0, NULL, GTK_SORT_ASCENDING, FALSE);
break;
case COLUMN_SELECTED_TOGGLE_RESOLVED:
new_packet_list_toggle_resolved (w, col_id);
@ -819,7 +833,7 @@ create_view_and_model(void)
}
gtk_tree_view_append_column(GTK_TREE_VIEW(packetlist->view), col);
/* XXX Breaks the GTK+ API, but this is the only way to attach a signal to
* a GtkTreeView column header. See GTK bug #141937.
*/
@ -864,9 +878,7 @@ new_packet_list_clear(void)
/* XXX is this correct in all cases?
* Reset the sort column, use packetlist as model in case the list is frozen.
*/
gtk_tree_sortable_set_sort_column_id(GTK_TREE_SORTABLE(packetlist),
0, GTK_SORT_ASCENDING);
new_packet_list_sort_column(0, NULL, GTK_SORT_ASCENDING, FALSE);
}
void
@ -1023,8 +1035,8 @@ scroll_to_and_select_iter(GtkTreeModel *model, GtkTreeSelection *selection, GtkT
0.5, /* row_align determines where the row is placed, 0.5 means center */
0); /* The horizontal alignment of the column */
/* "cursor-changed" signal triggers new_packet_list_select_cb() */
/* which will update the middle and bottom panes. */
/* "cursor-changed" signal triggers new_packet_list_select_cb() */
/* which will update the middle and bottom panes. */
gtk_tree_view_set_cursor(GTK_TREE_VIEW(packetlist->view),
path,
NULL,
@ -1170,8 +1182,8 @@ new_packet_list_set_selected_row(gint row)
selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(packetlist->view));
gtk_tree_selection_select_iter (selection, &iter);
/* "cursor-changed" signal triggers new_packet_list_select_cb() */
/* which will update the middle and bottom panes. */
/* "cursor-changed" signal triggers new_packet_list_select_cb() */
/* which will update the middle and bottom panes. */
gtk_tree_view_set_cursor(GTK_TREE_VIEW(packetlist->view),
path,
NULL,
@ -1224,12 +1236,12 @@ new_packet_list_select_cb(GtkTreeView *tree_view, gpointer data _U_)
cf_select_packet(&cfile, row);
/* If searching the tree, set the focus there; otherwise, focus on the packet list */
if (cfile.search_in_progress && (cfile.decode_data || cfile.decode_data)) {
if (cfile.search_in_progress && (cfile.decode_data || cfile.decode_data)) {
gtk_widget_grab_focus(tree_view_gbl);
} else {
gtk_widget_grab_focus(packetlist->view);
}
/* Add newly selected frame to packet history (breadcrumbs) */
packet_history_add(row);
}
@ -1485,14 +1497,14 @@ mark_all_displayed_frames(gboolean set)
void
new_packet_list_mark_all_displayed_frames_cb(GtkWidget *w _U_, gpointer data _U_)
{
mark_all_displayed_frames(TRUE);
mark_frames_ready();
mark_all_displayed_frames(TRUE);
mark_frames_ready();
}
void
new_packet_list_unmark_all_displayed_frames_cb(GtkWidget *w _U_, gpointer data _U_)
{
mark_all_displayed_frames(FALSE);
mark_all_displayed_frames(FALSE);
mark_frames_ready();
}

View File

@ -629,9 +629,9 @@ packet_list_append_record(PacketList *packet_list, frame_data *fdata)
g_return_val_if_fail(PACKETLIST_IS_LIST(packet_list), -1);
newrecord = se_alloc(sizeof(PacketListRecord));
newrecord->columnized = FALSE;
newrecord->colorized = FALSE;
newrecord->fdata = fdata;
newrecord->columnized = FALSE;
newrecord->colorized = FALSE;
newrecord->fdata = fdata;
newrecord->physical_pos = PACKET_LIST_RECORD_COUNT(packet_list->physical_rows);
if (fdata->flags.passed_dfilter || fdata->flags.ref_time) {
@ -643,14 +643,11 @@ packet_list_append_record(PacketList *packet_list, frame_data *fdata)
PACKET_LIST_RECORD_APPEND(packet_list->physical_rows, newrecord);
if (packet_list->columnized) {
/* XXX, dissect? */
packet_list->columnized = FALSE;
}
packet_list->columnized = FALSE; /* XXX, dissect? */
/*
/*
* Issue a row_inserted signal if the model is connected
* and the row is vissible.
* and the row is visible.
*/
if((model)&&(newrecord->visible_pos!=-1))
packet_list_row_inserted(packet_list, newrecord->visible_pos);
@ -776,7 +773,15 @@ packet_list_column_contains_values(PacketList *packet_list, gint sort_col_id)
return FALSE;
}
static void
/* packet_list_dissect_and_cache_all()
* returns:
* TRUE if columnization completed;
* packet_list->columnized set to TRUE;
* FALSE: columnization did not complete (i.e., was stopped by the user);
* packet_list->columnized unchanged (i.e., FALSE).
*/
static gboolean
packet_list_dissect_and_cache_all(PacketList *packet_list)
{
PacketListRecord *record;
@ -809,47 +814,70 @@ packet_list_dissect_and_cache_all(PacketList *packet_list)
main_window_update();
for (progbar_loop_var = 0; progbar_loop_var < progbar_loop_max; ++progbar_loop_var) {
record = PACKET_LIST_RECORD_GET(packet_list->physical_rows, progbar_loop_var);
packet_list_dissect_and_cache_record(packet_list, record, TRUE, FALSE);
record = PACKET_LIST_RECORD_GET(packet_list->physical_rows, progbar_loop_var);
packet_list_dissect_and_cache_record(packet_list, record, TRUE, FALSE);
/* Create the progress bar if necessary.
We check on every iteration of the loop, so that it takes no
longer than the standard time to create it (otherwise, for a
large file, we might take considerably longer than that standard
time in order to get to the next progress bar step). */
if (progbar == NULL)
progbar = delayed_create_progress_dlg("Construct", "Columns",
TRUE, &progbar_stop_flag,
&progbar_start_time, progbar_val);
/* Create the progress bar if necessary.
We check on every iteration of the loop, so that it takes no
longer than the standard time to create it (otherwise, for a
large file, we might take considerably longer than that standard
time in order to get to the next progress bar step). */
if (progbar == NULL)
/* Note: The following may call gtk_main_iteration() which will */
/* allow certain "interupts" to happen during this code. */
/* (Note that the progress_dlg window is set to "modal" */
/* so that clicking on other windows is disabled). */
progbar = delayed_create_progress_dlg("Construct", "Columns",
TRUE, &progbar_stop_flag,
&progbar_start_time, progbar_val);
if (progbar_loop_var >= progbar_nextstep) {
/* let's not divide by zero. We should never be started
* with count == 0, so let's assert that */
g_assert(progbar_loop_max > 0);
if (progbar_loop_var >= progbar_nextstep) {
/* let's not divide by zero. We should never be started
* with count == 0, so let's assert that */
g_assert(progbar_loop_max > 0);
progbar_val = (gfloat) progbar_loop_var / progbar_loop_max;
progbar_val = (gfloat) progbar_loop_var / progbar_loop_max;
if (progbar != NULL) {
g_snprintf(progbar_status_str, sizeof(progbar_status_str),
"%u of %u frames", progbar_loop_var+1, progbar_loop_max);
update_progress_dlg(progbar, progbar_val, progbar_status_str);
if (progbar != NULL) {
g_snprintf(progbar_status_str, sizeof(progbar_status_str),
"%u of %u frames", progbar_loop_var+1, progbar_loop_max);
/* Note: See comment above re use of gtk_main_iteration() */
update_progress_dlg(progbar, progbar_val, progbar_status_str);
}
progbar_nextstep += progbar_quantum;
}
progbar_nextstep += progbar_quantum;
}
if (progbar_stop_flag) {
/* Well, the user decided to abort the resizing... */
break;
}
if (progbar_stop_flag) {
/* Well, the user decided to abort ... */
break;
}
}
/* We're done resizing the columns; destroy the progress bar if it
was created. */
/* We're done; destroy the progress bar if it was created. */
if (progbar != NULL)
destroy_progress_dlg(progbar);
destroy_progress_dlg(progbar);
if (progbar_stop_flag) {
return FALSE; /* user aborted before columnization completed */
}
packet_list->columnized = TRUE;
return TRUE;
}
/* packet_list_do_packet_list_dissect_and_cache_all()
* returns:
* TRUE: if columnization not needed or columnization completed;
* FALSE: columnization did not complete (i.e., stopped by the user)
*/
gboolean
packet_list_do_packet_list_dissect_and_cache_all(PacketList *packet_list, gint sort_col_id)
{
if (!packet_list_column_contains_values(packet_list, sort_col_id)) {
return packet_list_dissect_and_cache_all(packet_list);
}
return TRUE;
}
static void
@ -874,9 +902,6 @@ packet_list_sortable_set_sort_column_id(GtkTreeSortable *sortable,
if(PACKET_LIST_RECORD_COUNT(packet_list->physical_rows) == 0)
return;
if (!packet_list_column_contains_values(packet_list, sort_col_id))
packet_list_dissect_and_cache_all(packet_list);
packet_list_resort(packet_list);
/* emit "sort-column-changed" signal to tell any tree views
@ -933,7 +958,7 @@ packet_list_compare_custom(gint sort_id, PacketListRecord *a, PacketListRecord *
/* Attempt to convert to numbers */
double num_a = atof(a->fdata->col_text[sort_id]);
double num_b = atof(b->fdata->col_text[sort_id]);
if (num_a < num_b)
return -1;
else if (num_a > num_b)
@ -958,17 +983,12 @@ packet_list_compare_records(gint sort_id, PacketListRecord *a,
g_assert(b->fdata->col_text[sort_id]);
if(a->fdata->col_text[sort_id] == b->fdata->col_text[sort_id])
return 0; /* not always NULL, but anyway no need to call strcmp() */
return 0; /* no need to call strcmp() */
if((a->fdata->col_text[sort_id]) && (b->fdata->col_text[sort_id])) {
if (cfile.cinfo.col_fmt[sort_id] == COL_CUSTOM) {
return packet_list_compare_custom (sort_id, a, b);
}
return strcmp(a->fdata->col_text[sort_id], b->fdata->col_text[sort_id]);
} else
return (a->fdata->col_text[sort_id] == NULL) ? -1 : 1;
g_assert_not_reached();
if (cfile.cinfo.col_fmt[sort_id] == COL_CUSTOM) {
return packet_list_compare_custom (sort_id, a, b);
}
return strcmp(a->fdata->col_text[sort_id], b->fdata->col_text[sort_id]);
}
static gint
@ -1091,7 +1111,13 @@ packet_list_dissect_and_cache_record(PacketList *packet_list, PacketListRecord *
gboolean create_proto_tree;
union wtap_pseudo_header pseudo_header; /* Packet pseudo_header */
guint8 pd[WTAP_MAX_PACKET_SIZE]; /* Packet data */
/* XXX: Does it work to check if the record is already columnized/colorized ?
* i.e.: test record->columnized and record->colorized and just return
* if they're both TRUE.
* Note: Part of the patch submitted with Bug #4273 had code to do this but it
* was commented out in the patch and was not included in SVN #33011.
*/
fdata = record->fdata;
if (dissect_columns)
@ -1121,7 +1147,7 @@ packet_list_dissect_and_cache_record(PacketList *packet_list, PacketListRecord *
if (dissect_columns) {
/* "Stringify" non frame_data vals */
epan_dissect_fill_in_columns(&edt, FALSE, FALSE /* fill_fd_colums */);
epan_dissect_fill_in_columns(&edt, FALSE, FALSE /* fill_fd_columns */);
for(col = 0; col < cinfo->num_cols; ++col) {
/* Skip columns based om frame_data because we already store those. */
@ -1264,7 +1290,7 @@ packet_list_get_widest_column_string(PacketList *packet_list, gint col)
guint widest_column_len = 0;
if (!packet_list->columnized)
packet_list_dissect_and_cache_all(packet_list);
packet_list_dissect_and_cache_all(packet_list); /* XXX: need to handle case of "incomplete" ? */
for(vis_idx = 0; vis_idx < PACKET_LIST_RECORD_COUNT(packet_list->visible_rows); ++vis_idx) {
record = PACKET_LIST_RECORD_GET(packet_list->visible_rows, vis_idx);

View File

@ -116,6 +116,7 @@ gboolean packet_list_visible_record(PacketList *packet_list, GtkTreeIter *iter);
gint packet_list_append_record(PacketList *packet_list, frame_data *fdata);
void packet_list_change_record(PacketList *packet_list, guint row, gint col, column_info *cinfo);
void packet_list_dissect_and_cache_iter(PacketList *packet_list, GtkTreeIter *iter, gboolean dissect_columns, gboolean dissect_color);
gboolean packet_list_do_packet_list_dissect_and_cache_all(PacketList *packet_list, gint sort_col_id);
void packet_list_reset_colorized(PacketList *packet_list);
const char* packet_list_get_widest_column_string(PacketList *packet_list, gint col);