Wireshark-bugs: [Wireshark-bugs] [Bug 8787] 9P dissector - compute fids on first visit
Comment # 11
on bug 8787
from Dominique Martinet
(In reply to comment #10)
> I took a little closer look today. I think that separating all that logic
> into that firstpass function is probably a complicated and risky way to do
> things. It effectively means we've got 2 pieces of code parsing the
> messages far enough to process the fid. You can imagine that 3 years from
> now someone might fix an offset increment mistake in one but not the other.
>
> 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.
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)
I'd claim that the protocol shouldn't change too much, but I'm painfully 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.
You are receiving this mail because:
- You are watching all bug changes.
