[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2581





--- Comment #6 from Martin Mathieson <martin.r.mathieson@xxxxxxxxxxxxxx>  2008-06-23 03:03:32 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> 
> I'm happy that you liked the idea as I think it's a useful enhancement due to
> the large number of protocols Wireshark already supports, and thanks a lot for
> your comments.
> 
> see my comments below.
> 
> > I like this idea a lot, but I've noted some problems while playing with the
> > current patch.
> > 
> > There are lots of places this callback function should be called, or rather it
> > should be a replacement for the function used everywhere else.
> > 
> 
> I'm aware of that, but I just wrote this patch as a "pilot". I’m going to
> upload another patch soon.
> 

OK, just wanted to be sure.  The other part would be to call the existing one
to get the useful colouring (apologies if you already did this - in my build
this doesn't work properly everywhere, I think due to the gtk version I have
installed).

> 
> > Rather than showing the current position in longer list of filter fields, might
> > it be possible to restrict the list to those filters matching the current
> > prefix?
> 
> 
> Well, I used the mechanism that is been used by Wireshark to loads the
> protocols and their fields into the memory; Wireshark loads the protocols into
> a list and it also load the fields into another list for each protocol, which I
> don't think is a good way to load the protocols and their fields into the
> memory, a better way is to use a Trie tree for that.
> 
> I can show only the filters that match the current prefix, by creating a Trie
> tree and loading the protocols and their fields again into that tree.
> 

Yes, what I noticed was that when I typed in sip.auth.  I was offered all of
the filters I'd hope for, but also one from the protocol before and (I think)
some from after (but I couldn't swear to the after part and have reverted the
old patch for now).

> 
> > I tried pressing down, hoping that the first visible list item would get
> > selected, but it didn't.
> > 
> > This popup list doesn't appear when you delete characters back to the last dot.
> 
> 
> Trie tree will help a lot in taking care of a situation when a user deletes
> characters.
> 
> 
> > Once I clicked on a choice, it was selected immediately.  I'd have preferred it
> > to wait until I've pressed enter.
> > 
> > This is just my opinion, but the ideal behaviour for me would be totally
> > keyboard-driver, so I could press down to browse them, enter to select one,
> > escape to dismiss the list and return control to the text control.
> > 
> 
> regarding the keyboard and the crash issues I will take care of that in my next
> patch.
> 
> 
> Do you think I should ignore the way Wireshark loads the protocols and their
> fields into memory and that I create a Trie tree to load them again?
> 
> 

I'm not sure what you mean here (or how trie trees work).  I'd guess that
getting the fields as you did before using proto_registrar_get_nth(),
proto_get_first_protocol_field() and proto_get_next_protocol_field() will be
quick enough, and that it wouldn't be worth making assumptions about how
they're stored in memory.


I'll take a look at your next patch.

> 
> > 
> > I did see a couple of crashes while running under gdb:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 1111388256 (LWP 8242)]
> > filter_te_focus_out_cb (filter_te=0x8a900b0, event=0x8342db8, data=0x8ab0ce0)
> > at filter_dlg.c:1404
> > 1404      if(GTK_IS_WIDGET(data)) {
> > (gdb) bt
> > #0  filter_te_focus_out_cb (filter_te=0x8a900b0, event=0x8342db8,
> > data=0x8ab0ce0) at filter_dlg.c:1404
> > #1  0x41b59bee in gtk_marshal_BOOLEAN__VOID () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #2  0x41e878bd in g_closure_invoke () from /opt/gnome/lib/libgobject-2.0.so.0
> > #3  0x41e98243 in g_signal_connect_closure_by_id () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #4  0x41e9988f in g_signal_emit_valist () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #5  0x41e99c95 in g_signal_emit () from /opt/gnome/lib/libgobject-2.0.so.0
> > #6  0x41c448d8 in gtk_widget_get_default_style () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #7  0x41c505b2 in gtk_window_set_auto_startup_notification () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #8  0x08342db8 in ?? ()
> > #9  0x00000000 in ?? ()
> > (gdb)      
> > 
> > 
> > A second crash looked like this:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > [Switching to Thread 1111388256 (LWP 8369)]
> > 0x41ea0479 in g_type_check_instance_is_a () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > (gdb) bt
> > #0  0x41ea0479 in g_type_check_instance_is_a () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #1  0x08084651 in filter_string_te_key_pressed_cb (filter_te=0x8a8cc90,
> > event=0x8342d38) at filter_dlg.c:1513
> > #2  0x41b59bee in gtk_marshal_BOOLEAN__VOID () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #3  0x41e878bd in g_closure_invoke () from /opt/gnome/lib/libgobject-2.0.so.0
> > #4  0x41e98243 in g_signal_connect_closure_by_id () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #5  0x41e9988f in g_signal_emit_valist () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #6  0x41e99c95 in g_signal_emit () from /opt/gnome/lib/libgobject-2.0.so.0
> > #7  0x41c448d8 in gtk_widget_get_default_style () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #8  0x41c53bb7 in gtk_window_propagate_key_event () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #9  0x41c56a0c in gtk_window_activate_key () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #10 0x41b59bee in gtk_marshal_BOOLEAN__VOID () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #11 0x41e860c7 in g_value_set_static_boxed () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #12 0x41e878bd in g_closure_invoke () from /opt/gnome/lib/libgobject-2.0.so.0
> > #13 0x41e98893 in g_signal_connect_closure_by_id () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #14 0x41e9988f in g_signal_emit_valist () from
> > /opt/gnome/lib/libgobject-2.0.so.0
> > #15 0x41e99c95 in g_signal_emit () from /opt/gnome/lib/libgobject-2.0.so.0
> > #16 0x41c448d8 in gtk_widget_get_default_style () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #17 0x41b5363a in gtk_propagate_event () from
> > /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #18 0x41b54857 in gtk_main_do_event () from /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #19 0x41d5b58a in gdk_add_client_message_filter () from
> > /opt/gnome/lib/libgdk-x11-2.0.so.0
> > #20 0x42158abd in g_main_context_dispatch () from
> > /opt/gnome/lib/libglib-2.0.so.0
> > #21 0x4215bcbf in g_main_context_check () from /opt/gnome/lib/libglib-2.0.so.0
> > #22 0x4215c069 in g_main_loop_run () from /opt/gnome/lib/libglib-2.0.so.0
> > #23 0x41b54cd4 in gtk_main () from /opt/gnome/lib/libgtk-x11-2.0.so.0
> > #24 0x0808a8fa in main (argc=0, argv=0xbf8a1c78) at main.c:2584
> > (gdb)                                   
> > 
> 


-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.