OpenOCD
|
OpenOCD is to some extent a "self service" open source project, so to contribute, you must follow the standard procedures to have the best possible chance to get your changes accepted.
The procedure to create a patch is essentially:
Your patch (or commit) should be a "good patch": focus it on a single issue, and make it easily reviewable. Don't make it so large that it's hard to review; split large patches into smaller ones (this will also help to track down bugs later). All patches should be "clean", which includes preserving the existing coding style and updating documentation as needed. When adding a new command, the corresponding documentation should be added to doc/openocd.texi
in the same commit. OpenOCD runs on both Little Endian and Big Endian hosts so the code can't count on specific byte ordering (in other words, must be endian-clean).
There are several additional methods of improving the quality of your patch:
Runtime testing with Valgrind Memcheck
This helps to spot memory leaks, undefined behaviour due to uninitialized data or wrong indexing, memory corruption, etc.
Clang Static Analyzer
Using this tool uncovers many different kinds of bugs in C code, with problematic execution paths fully explained. It is a part of standard Clang installation.
To generate a report, run this in the OpenOCD source directory:
Runtime testing with sanitizers
Both GCC and LLVM/Clang include advanced instrumentation options to detect undefined behaviour and many kinds of memory errors. Available with -fsanitize=*
command arguments.
Example usage:
Please consider performing these additional checks where appropriate (especially Clang Static Analyzer for big portions of new code) and mention the results (e.g. "Valgrind-clean, no new Clang analyzer warnings") in the commit message.
Say in the commit message if it's a bugfix (describe the bug) or a new feature. Don't expect patches to merge immediately for the next release. Be ready to rework patches in response to feedback.
Add yourself to the GPL copyright for non-trivial changes.
refs/for/master
on the Gerrit server, even if you plan to use several local branches for different topics. It is possible because for/master
is not a traditional Git branch.specify touched area
' should identify the main part or subsystem the patch touches. Further reading: http://www.coreboot.org/Git
OpenOCD source code includes the script checkpatch to let developers to verify their patches before submitting them for review (see Submitting patches to the OpenOCD Gerrit server).
Every patch for OpenOCD project that is submitted for review on Gerrit is tested by Jenkins. Jenkins will run the checkpatch script to analyze each patch. If the script highlights either errors or warnings, Gerrit will add the score "-1" to the patch and maintainers will probably ignore the patch, waiting for the developer to send a fixed version.
The script checkpatch verifies the SPDX tag for new files against a very short list of license tags. If the license of your contribution is not listed there, but compatible with OpenOCD license, please alert the maintainers or add the missing license in the first patch of your patch series.
The script checkpatch has been originally developed for the Linux kernel source code, thus includes specific tests and checks related to Linux coding style and to Linux code structure. While the script has been adapted for OpenOCD specificities, it still includes some Linux related test. It is then possible that it triggers sometimes some false positive!
If you think that the error identified by checkpatch is a false positive, please report it to the openocd-devel mailing list or prepare a patch for fixing checkpatch and send it to Gerrit for review.
There are exceptional cases in which you need to skip some of the tests from checkpatch in order to pass the approval from Gerrit.
For example, a patch that modify one line inside a big comment block will not show the beginning or the end of the comment block. This can prevent checkpatch to detect the comment block. Checkpatch can wrongly consider the modified comment line as a code line, triggering a set of false errors.
Only for exceptional cases, it is allowed to submit patches to Gerrit with the special field 'Checkpatch-ignore:' in the commit message. This field will cause checkpatch to ignore the error types listed in the field, only for the patch itself. The error type is printed by checkpatch on failure. For example the names of Windows APIs mix lower and upper case chars, in violation of OpenOCD coding style, triggering a 'CAMELCASE' error:
Adding in the commit message of the patch the line:
will force checkpatch to ignore the CAMELCASE error.
The code review is intended to take as long as a week or two to allow maintainers and contributors who work on OpenOCD only in their spare time opportunity to perform a review and raise objections.
With Gerrit much of the urgency of getting things committed has been removed as the work in progress is safely stored in Gerrit and available if someone needs to build on your work before it is submitted to the official repository.
Another factor that contributes to the desire for longer cool-off times (the time a patch lies around without any further changes or comments), it means that the chances of quality regression on the master branch will be much reduced.
If a contributor pushes a patch, it is considered good form if another contributor actually approves and submits that patch.
It should be noted that a negative review in Gerrit ("-1" or "-2") may (but does not have to) be disregarded if all conditions listed below are met:
All OpenOCD patches can be reviewed here.
From the main Review page select the patch you want to review and click on that patch. On the appearing page select the download method (top right). Apply the patch. After building and testing you can leave a note with the "Reply" button and mark the patch with -1, 0 and +1.