On Sat, Apr 26, 2008 at 07:07:27PM -0500, Jess Balint wrote: > Hi, I've attached a patch to add support for field info in the MySQL > dissector. I've also cleaned up a few things and fixed a small problem > in the state tracking I added in a previous patch.
Looks nice, a few questions/remarks though: - Do you have a sample trace that you could upload to the sample captures page on wiki.wireshark.org? It will help running regression tests. > @@ -30,6 +30,7 @@ > * > * the protocol spec at > * http://public.logicacmg.com/~redferni/mysql/MySQL-Protocol.html > + * http://forge.mysql.com/wiki/MySQL_Internals_ClientServer_Protocol > * and MySQL source code Is the old URL still relevant or can it be removed? > + if (tree) { > + ti= proto_tree_add_item(tree, proto_mysql, tvb, offset, -1, > FALSE); > + mysql_tree= proto_item_add_subtree(ti, ett_mysql); > + proto_tree_add_item(mysql_tree, hf_mysql_packet_length, tvb, > + offset, 3, TRUE); > + } > + offset+= 3; ... > - if (tree) { > - ti= proto_tree_add_item(tree, proto_mysql, tvb, offset, -1, > FALSE); > - mysql_tree= proto_item_add_subtree(ti, ett_mysql); > - proto_tree_add_item(mysql_tree, hf_mysql_packet_length, tvb, > - offset, 3, TRUE); > - } > - offset+= 3; Why do you move the stuff up? > + { &hf_mysql_fld_type, > + { "Type", "mysql.field.type", > + FT_STRING, BASE_DEC, NULL, 0x0, > + "Field: type", HFILL }}, ... > + proto_tree_add_string(tree, hf_mysql_fld_type, tvb, offset, -1, > + val_to_str(tvb_get_guint8(tvb, offset), > type_constants, "Unknown (%u)")); Why do you use val_to_str here instead of VALS(type_constants) in the definition of hf_mysql_fld_type? Thanks for doing this! Ciao Joerg -- Joerg Mayer <[EMAIL PROTECTED]> We are stuck with technology when what we really want is just stuff that works. Some say that should read Microsoft instead of technology. _______________________________________________ Wireshark-dev mailing list Wireshark-dev@wireshark.org http://www.wireshark.org/mailman/listinfo/wireshark-dev
