Comment # 1
on bug 8961
from Evan Huus
Hi Linas, thanks for the patch. I've had a chance to give it a quick review and
I have a few notes:
Q: "checkAPI.pl doesn't like the unicode degree symbol, I don't know what to
do..."
A: As noted in doc/README.developer:
Don't use non-ASCII characters in source files; not all compiler
environments
will be using the same encoding for non-ASCII characters. [...] This causes
source files using [...] UTF-8 to fail to compile.
In dissect_target, the check you do "if (rpt_tree == seg_tree)" will not always
work the way you expect. Wireshark can call for dissection with a NULL tree
value, in which case all of the proto_* calls are no-ops that also return NULL.
In this case, even if the sub-tree path was hit, rpt_tree == seg_tree == NULL
and the check will succeed (the only things depending on it at that point are
also no-ops, but it's a subtle bug if you ever add more complex logic based on
that choice).
The Dwell Segment Existence Masks are defined oddly, as they overlap bits (and
the n*8+x style is unusual). A comment would be helpful, even if it just points
to the relevant section of the spec.
Use G_GINT64_CONSTANT(0x1U) instead of 0x1ULL. Also from doc/README.developer:
When specifying an integral constant that doesn't fit in 32 bits, don't use
"LL" at the end of the constant - not all compilers use "LL" for that.
Instead, put the constant in a call to the "G_GINT64_CONSTANT()" macro.
Don't use col_add_fstr with a format string of %s, just use col_add_str.
There are a few places where the indentation is not consistent. Adding the
correct modelines (https://www.wireshark.org/tools/modelines.html) might help.
Your loop "while (offset < pkt_size)" could run forever if pkt_size is
MAX_UINT32 (or close to that). Offset will overflow between checks and never be
greater than it.
You are receiving this mail because:
- You are watching all bug changes.
