https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5013
Srinivasa Pradeep <sippyemail-wireshark@xxxxxxxxx> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #5287| |review_for_checkin?
Flag| |
--- Comment #13 from Srinivasa Pradeep <sippyemail-wireshark@xxxxxxxxx> 2010-10-12 01:47:12 PDT ---
Created an attachment (id=5287)
--> (https://bugs.wireshark.org/bugzilla/attachment.cgi?id=5287)
Patch based on revision 34437
Hi,
I have recreated the patch.
Following is my response to your comments.
>BTW, the checks like "if(fec_tree == NULL) return" could be simplified or
>mostly removed: the tree will either be NULL (at the beginning of the function)
>or not. If it's not NULL, proto_tree_add_subtree() won't generate a NULL tree
>either. Either way, checking for the tree being NULL is strictly optional at
>this point.
The point is to make it consistent. For eg: you see the similar
check in other places as well. So why remove it only here?
>You should probably also remove checks like:
>
>+ if ( (vc_len > 1) && ( rem > 1 ) ) { /* there is enough room
>for TAII */
>[...]
>+ } else {
>+ proto_tree_add_text(fec_tree,tvb,offset , 2 +vc_len,
>"Generalized FEC: TAII size format error");
>+ return;
>+ }
Again being consistent. I noticed that similar checks were made
for eg: in the VC_FEC tlv decoding case as well.
therefore I have the check in the dissection of the new FEC as well.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
