Wireshark-bugs: [Wireshark-bugs] [Bug 8787] 9P dissector - compute fids on first visit

Date: Thu, 13 Jun 2013 21:55:31 +0000

Comment # 12 on bug 8787 from
(In reply to comment #11)
> (In reply to comment #10)
> > Why not just put some "if not first pass, skip fid addition stuff" in the
> > existing dissection logic?
> 
> I did this because there was a silly "if (!tree) return" at the start of the
> function, and there are additems to tree functions all around below... And
> unless you tell me that proto_tree_add_whatever functions behave properly
> (i.e. do nothing) if main tree argument is null, it really sounds like a
> pain to check all the time.. Well I guess a wrapper would be ok.

The proto_tree_add_whatever functions will behave properly. :-) In fact there
has been some effort to remove if(tree)'s from the code (at least those which
make the code unreadable and those which cause bugs because columns or expert
infos or other such things are performed inside an if(tree) check).

> Second thing is that there's more common factors in the firstpass tree than
> in the other one, and I'm silly about code factorisation :P (also avoiding
> ten "if (firstpass)" checks, especially when I'm reusing the same variables
> all the time even inside one code path, so it wouldn't be easy to just do
> the firstpass after reading the whole packet without adding a few more - I'm
> bad with variable names if you hadn't noticed)

(Yeah, I contemplated complaining about that variable named "tmp" ;-) )

> I'd claim that the protocol shouldn't change too much, but I'm painfully

True enough; I was thinking more about someone finding a bug in one place and
not fixing the other corresponding spot.

> aware how right you are :/ so I guess I'll let this sit for a while until I
> find time to consider something that doesn't look too bad.

It's also possible that one of the other developers has an opinion different
than mine...


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