Guidelines and HOWTOs/Code Checking
There are a lot of ways to find bugs in KDE code. Increasingly, KDE developers have started to use automated tools. You can use some of those tools to improve your own code.
The KDE 'Krazy' Checker
KDE developers have a simple set of tests that are collectively known as "Krazy". These tests were originally developed to be run as part of a larger set of tests on a machine known as http://www.englishbreakfastnetwork.org, or EBN for short. You can see the results of running the various tests on EBN (at http://www.englishbreakfastnetwork.org/krazy/).
You can also run the tests yourself. To do this, you need to obtain a copy of the code from trunk/quality/krazy2 and install them. You can then test either a single file (using the krazy2 application) or a whole tree, including subdirectories (using the krazy2all application).
How Krazy works
The Krazy tests are essentially a form of static analysis - they check the source code, but not how it runs.
Krazy exists as a framework comprising a number of different test runners, and a set of plugins. The test runners are called krazy2, krazy2all, and krazy2ebn. The test runners just call one or more plugins on the appropriate code, and format the results for display.
At this stage, most of the test runners are written in perl, however one is written in C++ (using Qt) and it is quite possible to add your own tests, or to modify a test - all sources are provided.
Krazy needs to be installed before use. Krazy has two different ways to be installed - you can either modify the krazy2/install.sh script and run it, or follow the instructions in the krazy2/INSTALL.txt file. I recommend the second.
You may need to install additional perl modules like XML::LibXML, here is how:
linux-pudb:~/krazy2 # ./install.sh MakeMaker FATAL: prerequisites not found. Tie::IxHash not installed XML::LibXML not installed
Please install these modules first and rerun 'perl Makefile.PL'. linux-pudb:~/krazy2 # perl -mCPAN -e CPAN::shell
You may have to answer 25 useless questions here. In this case, just press ENTER 25 times. Then you go on like this:
cpan> install XML::LibXML
The Tie::IxHash I installed from the distro repository (libtie-ixhash-perl) and also the perl-doc package is needed to install it.
Krazy comes with a particularly good man page, which gives you the various options and a usage example. The file is generated on installation. This is definitely recommended reading!
As noted above, there are three test runners - krazy2, krazy2ebn and krazy2all. If you are trying to check a single file, then krazy2 is the right tool. If you are trying to check a source tree (say, an application or a whole subversion module), then krazy2all is more useful. krazy2all doesn't have a man page, but you can get a list of the options with krazy2all --help. You can also use krazy2 to get information on the various plugins, which can help you understand more about krazy2all.
krazy2ebn is the tool that runs over the KDE codebase on the EBN and should not be run locally. However, please see Controlling Krazy on the EBN below to learn how you can control which plugins are run, and what files are processed by the krazy2ebn program on the EBN machine.
Remember that Krazy doesn't change your code - it only examines it. So you can safely experiment with running Krazy checks until you are confident that you understand what is happening.
Equally, that means that Krazy doesn't fix problems - it only tries to report them. Understanding what is being reported, and how to fix it, is up to you. You should also remember the KDE commit policy about not committing code that you don't understand. So fixing a spelling error in a comment is pretty safe, but blindly changing code to stop explicit constructor warnings from Krazy is not a good idea.
The Krazy plugins support the following list of in-code directives:
- //krazy:skip - no Krazy tests will run on this file.
- //krazy:excludeall=<name1[,name2,...,nameN]> - the Krazy tests name1, etc will not be run on this file. Multiple occurrences of krazy:excludeall are allowed.
- //krazy:exclude=<name1[,name2,...,nameN]> - the Krazy tests name1, etc. will not be run on the line where this directive is found (see the next section below for more information).
Note that these directives must be C++ style comments that can be put anywhere in the file desired (except embedded within C-style comments).
The Krazy tests are designed to minimise false positives (that is, alerts that do not represent real problems). However because most of the tests are conducted on a single line, there are some tests that might produce such a false positive. For example, code that does something like:
mystring += "/";
will be flagged by the doublequote_chars checker, because it is more efficient to add a single char, as shown below:
// note that we are using single quotes
// this is a char, not a char array
mystring += '/';
That same checker will produce a false positive for code that looks like:
mystring += "/";
You can suppress these false positives using a special comment format. To exclude a particular plugin from being run on a line of code, simply add a C++ comment containing the string "krazy:exclude=<plugin_name>". The plugins currently available can be found in the repository.
Specifically, for this plugin use "krazy:exclude=doublequote_chars".
lenstr = "0" + lenstr;
lenstr = "0" + lenstr; // krazy:exclude=doublequote_chars
Controlling Krazy on the EBN
This section describes how to use .krazy files to control the Krazy runs on the EBN. The .krazy files are used to tell Krazy to skip over specific sub-directories, or files; or to disable certain plugins within those modules and sub-directories.
To ignore a sub-directory within a module, say kdepim/kmail, use the IGNORESUBS directory within the kdepim/.krazy file, like so:
Or you can ignore a set of directories by specifying a comma-separated list:
To ignore files or directories within a module/subdir, specify a regular expression that matches the files to skip together with the SKIP directive. For example, to skip the directories kdepimlibs/kcal/libical, kdepimlibs/kcal/versit, and the kdepimlibs/kcal/fred.c file, use this directive within the kdepim/kcal/.krazy file:
Use the EXCLUDE directive to disable a list of plugins for all files within a module/subdir:
To override the EXCLUDE directive set from a .krazy file up in the directory hierarchy, use the CHECK command. For example, the component level .krazy file may EXCLUDE the copyright and license plugins, but those plugins can be re-enabled in a module/subdir with the CHECK directive like so:
In addition to the various Krazy tools, you can also get valuable assistance from the warnings that the compiler emits, especially if you enable additional warnings (per the documentation for your compiler), and also if you test with more than one compiler (e.g. if you can test on Linux with both GCC and the Intel compiler; or on Linux with GCC and also on Windows with the Microsoft compiler).
Low: Can be fixed by anyone with minimal risk of error.
Medium: Can be fixed by anyone with appropriate knowledge of C++ features involved, some testing advised.
High: Should only be fixed by maintainer/owner of code
Spelling errors in comments and strings should be fixed as they may show up later in API documentation, handbooks, etc. Misspelled strings make the translator's job harder. Please use US English.
Risk from Fixing: Low
Override inline with:
Do not assign QString::null or QString() to a QString. Instead use the .clear() method.
fileName = QString::null;
fileName = QString();
When returning an empty string from a method use "return QString()" When passing an empty string use "QString()".
Override inline with:
Message: QLatin1String issues
Risk from Fixing: Low to Medium
Some QString methods (like startsWith() and endsWith()) are more efficient if they are passed a QLatin1String, avoiding an implicit conversion from const char *.
A common false positive is with QByteArray which cannot take a QLatin1String. To exclude comment inline with:
Message: duplicate includes
Risk from Fixing: Low
The same file has been included twice, remove the second occurrence.
Message: include own header first
Message: include own _p header first
Risk from Fixing: Medium
The cpp file should include their own .h and _p.h headers first in the file (but below config.h). Move the includes to the correct position. You may need to adjust includes and forward declarations in other files as a result, the compiler will advise of these.
Use <..> to include installed headers.
To include Qt headers from installed headers.
Use include guards in headers with appropriately encoded macro names.
To exclude comment inline with: