Wireshark-bugs: [Wireshark-bugs] [Bug 5324] New dissector for adwin protocols

Date: Mon, 25 Oct 2010 00:22:50 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5324

--- Comment #9 from Thomas Boehne <TBoehne@xxxxxxxx> 2010-10-25 00:22:44 PDT ---
(In reply to comment #8)
> Checked in with a bunch of changes (including fixing a bunch of memory leaks).

Thanks for the checkin. I knew about the ep_-functions, but I did not know that
they were neccessary for strings handed to other functions that could free the
strings directly.

> Some comments:
> Why not add the 2 times in adwin-config using proto_tree_add_time() so they'll
> be filterable?

We could do that, but since that is an old firmware update protocol that is not
used in current devices, and since those fields are basically the file times of
updated files, I cannot think of a useful filter for those fields. But as you
said, it would not hurt to use proto_tree_add_time(). I'll keep it in mind for
the next version.

> You shouldn't need to set this, so I took it out:
>         pinfo->current_proto = "adwin_config";

IIRC, that was cut-and-pasted from an old version of the example plugin from
the README and was probably just outdated.

> I strengthened the heuristics in adwin-config.c and made packet-adwin.c a
> "new style" (semi-heuristic) dissector.  Please check my work here and if you
> can think of ways to further strengthen the heuristics, please supply a patch.

Looks good, just the return(FALSE) in dissect_adwin_config should probably be
return(0).

> (In particular the TCP heuristics in adwin-config are pretty weak.)

I know; but unfortunately the protocol does not have a lot more info. The only
other thing I could think of is checking if the destination MAC address is in
our MAC address range. It is an ugly hack, but the firmware update is usually
only used inside LANs and not in routed networks.

> BTW, comment 0 mentions a bug in another dissector: did you report that?

Yes, that was bug #5323. Turned out not to be a real bug.

Thanks
Thomas

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