Wireshark-dev: [Wireshark-dev] Static analysis and FT_STRING encodings
From: Evan Huus <eapache@xxxxxxxxx>
Date: Thu, 12 Apr 2012 13:47:54 -0400
Hi all,

This is my first post to the list, I just joined, so please be kind in pointing out the numerous mistakes I'm sure to make :)

While I've been working with Wireshark for a while now already, this is my first foray into actual upstream development. I wrote a couple of custom dissectors for a co-op placement last year, and have subsequently started caring for bug reports in Ubuntu (many thanks to Balint Reczey, the Debian maintainer, for some help there). Now I feel I'd like to contribute a little more.

I've read a bunch of documentation and checked out the Wireshark trunk. To get my feet wet, I started running CppCheck Static Analyzer [1] on the source. It's reported lots of interesting (but mostly quite minor) problems so far, but it's also raised a few questions:

1. Based on the documentation, the correct way to submit code normally is to file a bug and attach a patch for each unrelated issue. Given that so many of the CppCheck-reported issues are minor, one-line fixes, is it acceptable to file a single bug (with patch) for a group of issues, simply to save on bug spam?

2. Most dissectors add FT_STRINGs with an encoding value of 'ENC_ANSII | ENC_NA'. Based on the comments in epan/proto.h this doesn't make sense (they should be 'ENC_ASCII' only?), and they cause CppCheck to complain because both ENC_s are #defined to 0 (which makes or-ing them a no-op). Since 99% of the dissectors do it there must be a reason, but I can't for the life of me figure out what it is.

I'm sure I'll have more questions in a few days, but I'll hopefully also have some patches to submit by then!

Thanks,
Evan

[1] http://sourceforge.net/apps/mediawiki/cppcheck/index.php?title=Main_Page