Wireshark-bugs: [Wireshark-bugs] [Bug 2533] EBCDIC display for TN3270 packet
Date: Wed, 3 Jun 2009 13:10:40 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2533





--- Comment #6 from Jaap Keuter <jaap.keuter@xxxxxxxxx>  2009-06-03 13:10:39 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> > This patch needs more cleaning before it can become part of the main
> > repository.
> 
> And then some! It has only just graduated from naive-draft-from-spec to
> kind-of-works prototype.

Oke.

> > - Indentations must be consistent (don't mix tabs and spaces)
> > - Drop not required includes.
> > - For telnet.c don't put in a parameter in comments in telnet_add_text()
> > - "Remove memleaks", a must do item.
> > - Remove the C++ style comments, non-portable
> > - Drop the (Tor based?) preferences, they're not used
> 
> No problem.

Cool.

> > - Fix the header fields
> Not sure what you're referring to here. Can you clarify?

+    { &hf_tn3270_command_code,
+      { "Command Code",           "tn3270.command_code",
+      FT_UINT8, BASE_HEX, VALS(vals_command_codes), 0x0,
+        "Command Code", HFILL }},
This looks as expected (although FIELDDESCR could be NULL).

+    { &hf_tn3270_bsc,
+        {  "hf_tn3270_bsc", "hf_tn3270_bsc", 
+            FT_UINT8, BASE_HEX, NULL, 0x0,
+            "hf_tn3270_bsc", HFILL }},
This has a poor FIELDNAME, missing PROTOABBREV, poor FIELDABBREV and
FIELDDESCR.
Almost all header fields are like this.

See this template from README.developer:
        static hf_register_info hf[] = {
                { &hf_PROTOABBREV_FIELDABBREV,
                        { "FIELDNAME",           "PROTOABBREV.FIELDABBREV",
                        FIELDTYPE, FIELDBASE, FIELDCONVERT, BITMASK,
                        "FIELDDESCR", HFILL }
                }
        };


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