Wireshark-bugs: [Wireshark-bugs] [Bug 5048] VeriWave encapsulation type and WaveAgent protocol -
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5048
--- Comment #5 from Guy Harris <guy@xxxxxxxxxxxx> 2010-07-23 17:19:49 PDT ---
> Column types: I suppose these could be custom types, though we worked through
> the defined columns with our customers. These columns are just about required
> to sort out problems during network equipment testing, especially when testing
> a large number of WiFi clients (high-scale). My preference is to keep the
> defined columns so our customers can find them.
Perhaps what's needed is a way for dissectors to advertise particular fields as
"popular columns". We're trying to *reduce* the number of column types - we're
removing some in 1.4.x in favor of custom columns. This patch probably won't
go into 1.4.x anyway, so we could, I guess, add them in as column types now,
and replace them with custom columns if we come up with a UI for marking fields
as "popular columns".
> FPGA versions: There is a need to track different FPGA types because these
> types indicate what kind of test port we have, Ethernet or WiFi. Also, the
> FPGA type indicates the port vintage. The older FPGAs have a slightly different
> VWR file format.
There are several places where the FPGA version is used. The question is
whether the file_type field of the wtap structure needs to be one of them. The
only reason to call them different types of files would be:
1) if you want to show the user which type of file it is, e.g. in the
output of capinfos or in the Statistics -> Summary menu item;
2) if you plan to support *saving* files in that format, so that a file in
a given format is saved in the same format.
The port type isn't necessary for 2), as that can be determined from the
WTAP_ENCAP_ type, and isn't necessary for 1), either, as the encapsulation type
is printed/displayed. The only part that would matter would be the FPGA
version; that's somewhat like the file version number for other file formats.
You'd still store the FPGA version information in the private data structure I
referred to in my previous comment, so not setting the file type wouldn't mean
you wouldn't be tracking it at all.
> All the other items are clear and will be addressed in the next patch. Do you
> want me to send in multiple patches or just one?
I'm not sure what "multiple patches" means here. If you mean "do you want me
to leave the existing patch file in the bug, and then attach another patch to
be applied atop that patch", the answer is "no, I want you to remove the
existing patch and replace it with the new patch".
BTW, some other item:
check_col() is deprecated - just unconditionally call the col_set_/col_add_
functions.
In the hf_ entry for a field, if there's no blurb, use NULL, not a null string
("").
In vwr.c, you're doing stuff such as
frame_type = (s_510006_ptr[v22_W_FRAME_TYPE_OFF] << 24) |
(s_510006_ptr[v22_W_FRAME_TYPE_OFF + 1] << 16) |
(s_510006_ptr[v22_W_FRAME_TYPE_OFF + 2] << 8) |
(s_510006_ptr[v22_W_FRAME_TYPE_OFF + 3]);
That could be written as
frame_type = pntohl(&s_510006_ptr[v22_W_FRAME_TYPE_OFF]);
See the macros in wiretap/wtap-int.h.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
