[bugzilla-daemon@xxxxxxxxxxxxx]

Alexis La Goutte changed bug 8077

What	Removed	Added
Status	UNCONFIRMED	INCOMPLETE
Ever confirmed		1

Comment # 8 on bug 8077 from Alexis La Goutte

Hi Michele,

a quick review


use consistent white space rules ( dissectors use spaces, don't mix).

proto_register / proto_reg_handoff must be in the end of file 

Why create a new tvb (next_tvb) for decode MPLS TP Payload ?

+         default:
+           proto_tree_add_text(mplstpoam_tree, tvb, 0, 1, "MPLS-TP Default
packet, code %d", opcode...

or 
+         if (tvb_reported_length_remaining(tvb, 2) < tlv_offset) {
+           proto_tree_add_text(mplstp_ccm_tree, tvb, 0, -1, "Error processing
Message: there are %d
+           return;
+         } 

It is better to use expert_info 

+       next_tvb = tvb_new_subset_remaining(tvb, 2);
+       req_st = tvb_get_bits8(next_tvb, APS_REQ_ST_OFFSET, APS_REQ_ST_BIT);
+
+       proto_tree_add_item(mplstp_aps_tree, hf_mplstpoam_aps_req_st, tvb,
offset, 1, ENC_BIG_ENDIAN
+       
+
+       switch ( req_st )
+         {
+         case 0:
+           proto_tree_add_text(mplstp_aps_tree, tvb, offset, 1, "No Request
(NR)");
+           break;
+         case 1:
+           proto_tree_add_text(mplstp_aps_tree, tvb, offset, 1, "Do Not Revert
(DNR)");
+           break;
....
No need to creaet a new tvb (next_tvb) also look README.dev Guide about
value_string (and VALS), the switch (req_st.. is not needed)


Also it is possible to add a sample of MPLS-TP OAM ?

---

      You are receiving this mail because:

• You are watching all bug changes.