Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 8505] openSAFETY: New reassemble functionality, as well as minor bugfixes

[bugzilla-daemon@xxxxxxxxxxxxx]

Comment # 29 on bug 8505 from Evan Huus

Had 1/2 hour to do a quick skim. Nothing major left, just a few style questions
and misc.

- Instead of calling tvb_ensure_bytes_exist and wrapping it in a manual
TRY/CATCH block, you can instead just call tvb_bytes_exist which returns a
boolean.

- You do a lot of shifts of the style "( x >> 2 ) << 2" which I imagine is just
to zero the rightmost two bits of the value? If that's the case then it is
probably clearer and probably more efficient to just do "( x & 0xFC )".

- You do:

    frameCRC = tvb_get_guint8(...);
    if (dataLength > OSS_PAYLOAD_MAXSIZE_FOR_CRC8)
        frameCRC += (tvb_get_guint8(... + 1) << 8);

There's nothing strictly wrong with it, but if I'm understanding the intent
(that frameCRC can be either one or two bytes depending on dataLength) then
it's simpler and clearer to do:

    if (dataLength > OSS_PAYLOAD_MAXSIZE_FOR_CRC8)
        frameCRC = tvb_get_letohs(...)
    else
        frameCRC = tvb_get_guint8(...)

- Trunk has recently enabled -Wc++-compat as an error so it's not technically
compiling at the moment (just missing a cast I think).

Really, none of these issues are major and everything else looks great. Thanks
for all your hard work. I'll be happy to check in an updated patch without any
further review.

Cheers,
Evan

---

      You are receiving this mail because:

• You are watching all bug changes.