Wireshark-bugs: [Wireshark-bugs] [Bug 8947] Adding request/response tracking into COPS dissector
Michael Mann
changed
bug 8947
What |
Removed |
Added |
Attachment #11276 Flags |
review_for_checkin?
|
review_for_checkin-
|
Comment # 14
on bug 8947
from Michael Mann
Comment on attachment 11276 [details]
diff - 2013-07-24
I don't think this isn't going to work in all situations. I believe the
request/response should only be setup on the "first pass", which can't be done
because the cops_tree will always be NULL on the first pass. packet_info has
its own variable for detecting "first pass", pinfo->fd->flags.visited. That's
the only variable that should be used for detecting "first pass".
It also appears like there is an "extra dimension" in the "array of
request/response". Why is the cops_conv_info_t structure needed? Can't it
just be handled with the cops_call_t structure?
Also, the code to get the "COPS handle" is VERY hacky (handle_hfid is
completely unnecessary). I think you may be able to just do some of the
"conversation processing" at the time the handle is dissected, so you don't
need to "retrieve it retroactively". If "NULL trees" are preventing the
dissection, perhaps the better solution is to remove them. proto_tree_add_xxx
calls are protected already protected from NULL trees, it would just be an
optimization to add the NULL tree check. Typically this should only be done
around a large group of proto_tree_add_xxx calls that don't have other
"dissection functionality" (like creating conversation data).
You are receiving this mail because:
- You are watching all bug changes.