Wireshark-bugs: [Wireshark-bugs] [Bug 8859] Apply consistent naming convention to checksum displ

Date: Mon, 19 May 2014 17:52:26 +0000

Comment # 6 on bug 8859 from
(In reply to comment #5)
> (In reply to comment #4)
> >> checksum_bad and checksum_good are oddballs. I would expect them to act as
> >> a boolean for existence, but you actually have to check ip.checksum_bad==1
> >> for example. If checksumming is enabled, then PROTO.checksum_bad==1 implies
> >> PROTO.checksum_good==1 (and vice versa). Why do we have two fields for this?
> > 
> > It is for colorfilter ?
> 
> The color filter can also check for (not) PROTO.checksum_bad?
There is already check for bad...

> 
> >> proto_item*
> >> proto_tree_add_checksum(proto_tree *tree, checksum_t* checksum_hfs,
> >> tvbuff_t *tvb, gint start, gint length, guint32 (64?) chsum_expected);
> >>
> >> with checksum_t being a structure that contains all of the necessary hfs
> >> (checksum, good, bad, expected?, ett_ for "checksum tree", etc) and
> >> expert_info.
> >> It beats having to pass that many parameters into the function.
> >
> > +1 
> 
> Looks mostly fine to me. I forgot that since the values are just integers,
> the actual data type size does not matter. Minor detail: checksum_t * could
> be const. Do you want to delegate the creation of the checksum item to the
> dissector or to this helper function? The TCP dissector for example wants to
> treat certain invalid values as valid (0x0000, 0xffff?).
Good question...

> 
> >> Having a guint32 for checksum should cover most dissectors. [..]
> > 
> > use callback function for checksum ?
> 
> Nah, the checksum is usually already known, so it seems faster just to pass
> this directly instead of another level of indirection.
Oups ! it is correct !


You are receiving this mail because:
  • You are watching all bug changes.