add a custom column from the popup menu of the packet list

This issue was migrated from bug 4246 in our old bug tracker.

Original bug information:

Reporter: Yann Barilly
Status: RESOLVED FIXED
Product: Wireshark
Component: GTK+ UI
OS: Windows XP
Platform: x86
Version: 1.3.x (Experimental)

Attachments:

packet_list_popup_menu_add_custom_column.patch: add a custom column through popup menu
apply_as_column.patch: Apply as Column

Yann Barilly said:

Build Information: Version 1.3.2

Compiled with GTK+ 2.16.6, with GLib 2.22.2, with WinPcap (version unknown), with libz 1.2.3, without POSIX capabilities, with libpcre 7.0, with SMI 0.4.8, with c-ares 1.6.0, with Lua 5.1, without Python, with GnuTLS 2.8.5, with Gcrypt 1.4.4, with MIT Kerberos, with GeoIP, with PortAudio V19-devel (built Nov 17 2009), with AirPcap, with new_packet_list.

Running on Windows XP Service Pack 3, build 2600, with WinPcap version 4.1.1 (packet.dll version 4.1.0.1753), based on libpcap version 1.0 branch 1_0_rel0b (20091008), GnuTLS 2.8.5, Gcrypt 1.4.4, without AirPcap.

Built using Microsoft Visual C++ 9.0 build 30729

the purpose of this feature is to add simply a column from the packet list through the pop-up menu.

no bug detected

might be problems with not freed memory allocation. To check.

added enhancement oswindows uigtk versiondev labels

Yann Barilly said:

Created attachment 3968 add a custom column through popup menu

Stig Bjørlykke said:

Try adding a protocol as column.
Try adding a text item as column.
Try the functionality with the new packet list.

I get some crashes.


column_add_custom() has two unused arguments
please use same indent in functions.

Yann Barilly said:

(In reply to comment #2)  
> Try adding a protocol as column.  
> Try adding a text item as column.  
> Try the functionality with the new packet list.  
>  
> I get some crashes.  
>  
>  
> column_add_custom() has two unused arguments  
> please use same indent in functions.  
>  

arguments removed and indent modified on my platform

no crash founded while adding protocols (IP, UDP, http, ) or texts

can you send me some bugging traces please

Stig Bjørlykke said:

(In reply to comment #3)  
> no crash founded while adding protocols (IP, UDP, http, ) or texts  
>  
> can you send me some bugging traces please  

With old packet list:

In proto_custom_set() line 3424, the default case.  FT_PROTOCOL and FT_NONE is not handled here, and it should not be possible to add a custom column for this fields.  Maybe just disable the menu entry for illegal fields.

5   libepan.dylib                 	0x0000000100958608 proto_custom_set + 168 (proto.c:3424)
6   libepan.dylib                 	0x000000010093e3c3 col_custom_set_edt + 195 (column-utils.c:224)
7   libepan.dylib                 	0x00000001009424e6 epan_dissect_fill_in_columns + 38 (epan.c:244)
8   wireshark                     	0x0000000100012879 add_packet_to_packet_list + 473 (file.c:1250)
9   wireshark                     	0x0000000100012c57 rescan_packets + 727 (file.c:2181)
10  wireshark                     	0x0000000100050230 redissect_packets + 16
11  wireshark                     	0x0000000100053f9a apply_as_custom_column_cb + 106




With new packet list:

(wireshark:13952): Gtk-CRITICAL **: gtk_tree_view_column_get_width: assertion `GTK_IS_TREE_VIEW_COLUMN (tree_column)' failed

Stig Bjørlykke said:

Comment on attachment 3968 add a custom column through popup menu

This patch has a lot of unused code. I will provide a better patch.

Stig Bjørlykke said:

Created attachment 3979 Apply as Column

A better patch for "Apply as Column". Does this work for you?

Should we save the preferences when adding a new column from the packet details? Or should they be some sort of temporary?

Stig Bjørlykke said:

I have fixed the crash for protocol and text item (and ipv6) in revision 31016.

Now we only need new_packet_list_recreate() to make this work.

Stig Bjørlykke said:

Comment on attachment 3979 Apply as Column

Committed revision 31020.

Yann Barilly said:

(In reply to comment #6)  

> A better patch for "Apply as Column".  Does this work for you?  
>  
> Should we save the preferences when adding a new column from the packet  
> details?  
> Or should they be some sort of temporary?  
>  

sorry for the bad code
thanks for all the job you have done.

If we save it in the preferences, I think
it should be better to be able to remove the column as we added it.
Tag "remove this column" under "Apply as a column" for example.

But in my opinion, having seen the way people use it in my office,
it is better to save it.

If we go on this way (add "remove column")
I'll try to make a good patch without useless code.

Stig Bjørlykke said:

(In reply to comment #‍9)
> If we go on this way (add "remove column")
> I'll try to make a good patch without useless code.

I think the "remove column" should be a available from the packet list headings, so you can second-click the column and select remove. This way you don't have to find the correct field in the packet details to remove the column, and it can be used for built-in columns as well.

In this popup I'm also thinking about an easier access to the column preferences and maybe "sort ascending"/"sort descending" entries.

Yann Barilly said:

(In reply to comment #10)  
>  
> I think the "remove column" should be a available from the packet list  
> headings, so you can second-click the column and select remove.  This way you  
> don't have to find the correct field in the packet details to remove the  
> column, and it can be used for built-in columns as well.  
>  

I agree but I didn't speak about it because as far as I know there is no popup for the column title bar for the moment and I've no time to develop that.

> In this popup I'm also thinking about an easier access to the column  
> preferences and maybe "sort ascending"/"sort descending" entries.  

That is a good idea.

Anders Broman said:

Marking as resolved as I think this is implemented now.

closed