https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5051
--- Comment #6 from Bill Meier <wmeier@xxxxxxxxxxx> 2010-08-06 09:54:23 EDT ---
OK: I've looked at the patch in more detail:
My last (I think) comment on the patch:
To be honest, I'm not really thrilled by the use of /* ... */ to comment out
parts of the code:
Eg:
/* if (bacapp_seq > 0) /* Can't handle continuation segments, so just treat as
data */
/* {
proto_tree_add_text(bacapp_tree, tvb, offset, 0, "(continuation)");
return offset;
}
else
{ */
/* Service ACK follows... */
return fConfirmedServiceAck (tvb, pinfo, bacapp_tree, offset, svc);
/* } */
}
I find the above rather unreadable (not to mention that the gcc compiler will
produce a warning message: "/*" within comment).
Another example:
frag_msg = fragment_add_seq_check(tvb, data_offset, pinfo,
/* frag_msg = fragment_add_seq_check(tvb, bacapp_seqno == 0 ? 0 : data_offset,
pinfo, */
bacapp_invoke_id, /* ID for fragments belonging together */
msg_fragment_table, /* list of message fragments */
msg_reassembled_table, /* list of reassembled messages */
bacapp_seqno, /* fragment sequence number */
tvb_reported_length_remaining(tvb, data_offset), /* fragment
length - to the end */
/* tvb_reported_length_remaining(tvb, bacapp_seqno == 0 ? 0 :
data_offset), fragment length - to the end */
This is somewhat more readable but still messy.
In both cases (and similar) if you feel that the original code should remain as
a comment, may I suggest:
#if 0
<orig code>
#endif
<replacement/new code>
#if 0
if (bacapp_seq > 0) /* Can't handle continuation segments, so just treat as
data */
{
proto_tree_add_text(bacapp_tree, tvb, offset, 0, "(continuation)");
return offset;
}
else
{
/* Service ACK follows... */
return fConfirmedServiceAck (tvb, pinfo, bacapp_tree, offset, svc);
}
#endif
/* Service ACK follows... */
return fConfirmedServiceAck (tvb, pinfo, bacapp_tree, offset, svc);
}
IMHO it's not really necessary in many cases to leave the old code. One can
always look at the svn diff if necessary.
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
