OpenOCD
Patch Guidelines
Attention
You can't send patches to the mailing list anymore at all. Nowadays you are expected to send patches to the OpenOCD Gerrit GIT server for a review.
If you already have a Gerrit account and want to try a different sign in method, please first sign in as usually, press your name in the upper-right corner, go to Settings, select Identities pane, press Link Another Identity button. In case you already have duplicated accounts, ask administrators for manual merging.
If you're behind a corporate wall with http only access to the world, you can still use these instructions!

Submitting patches to the OpenOCD Gerrit server

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:

  • make the changes
  • create a commit
  • send the changes to the Gerrit server for review
  • correct the patch and re-send it according to review feedback

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:

    mkdir build-scanbuild; cd build-scanbuild
    scan-build ../configure
    scan-build make CFLAGS="-std=gnu99 -I. -I../../jimtcl"
  • 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:

    mkdir build-sanitizers; cd build-sanitizers
    ../configure CC=clang CFLAGS="-fno-omit-frame-pointer \
    -fsanitize=address -fsanitize=undefined -ggdb3"
    make
    export ASAN_OPTIONS=detect_stack_use_after_return=1
    src/openocd -s ../tcl -f /path/to/openocd.cfg

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.

Step by step procedure

  1. Create a Gerrit account at: https://review.openocd.org
    1. Add a username to your profile. After creating the Gerrit account and signing in, you will need to add a username to your profile. To do this, go to 'Settings', and add a username of your choice. Your username will be required in step 3 and substituted wherever the string 'USERNAME' is found.
    2. Create an SSH public key following the directions on github: https://help.github.com/articles/generating-ssh-keys . You can skip step 3 (adding key to Github account) and 4 (testing) - these are useful only if you actually use Github or want to test whether the new key works fine.
    3. Add this new SSH key to your Gerrit account: go to 'Settings' > 'SSH Public Keys', paste the contents of ~/.ssh/id_rsa.pub into the text field (if it's not visible click on 'Add Key ...' button) and confirm by clicking 'Add' button.
  2. Clone the git repository, rather than just download the source:
    git clone git://git.code.sf.net/p/openocd/code openocd
    or if you have problems with the "git:" protocol, use the slower http protocol:
    git clone http://git.code.sf.net/p/openocd/code openocd
  3. Set up Gerrit with your local repository. All this does it to instruct git locally how to send off the changes.
    1. Add a new remote to git using Gerrit username:
      git remote add review ssh://USERNAME@review.openocd.org:29418/openocd.git
      git config remote.review.push HEAD:refs/for/master
      Or with http only:
      git remote add review https://USERNAME@review.openocd.org/p/openocd.git
      git config remote.review.push HEAD:refs/for/master
      The http password is configured from your gerrit settings - https://review.openocd.org/#/settings/http-password.
      Note
      If you want to simplify http access you can also add your http password to the url as follows:
      git remote add review https://USERNAME:PASSWORD@review.openocd.org/p/openocd.git
      All contributions should be pushed to 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.
    2. You will need to install this hook, we will look into a better solution:
      scp -p -P 29418 USERNAME@review.openocd.org:hooks/commit-msg .git/hooks/
      Or with http only:
      wget https://review.openocd.org/tools/hooks/commit-msg
      mv commit-msg .git/hooks
      chmod +x .git/hooks/commit-msg
      Note
      A script exists to simplify the two items above. Execute:
      tools/initial.sh <username>
      With <username> being your Gerrit username.
  4. Set up git with your name and email:
    git config --global user.name "John Smith"
    git config --global user.email "john@smith.org"
  5. Work on your patches. Split the work into multiple small patches that can be reviewed and applied separately and safely to the OpenOCD repository.
    while(!done) {
    work - edit files using your favorite editor.
    run "git commit -s -a" to commit all changes.
    run tools/checkpatch.sh to verify your patch style is ok.
    }
    Note
    use "git add ." before commit to add new files.
    check About checkpatch script for hint about checkpatch script
    Commit message template, notice the short first line. The field 'specify touched area' should identify the main part or subsystem the patch touches.
    specify touched area: short comment
    <blank line>
    Longer comments over several lines, explaining (where applicable) the
    reason for the patch and the general idea the solution is based on,
    any major design decisions, etc. Limit each comment line's length to 75
    characters; since 75 it's too short for a URL, you can put the URL in a
    separate line preceded by 'Link: '.
    <blank line>
    Signed-off-by: ...
    Examples:
    flash/nor/atsame5: add SAME59 support
    Add new device ID
    flash/nor: flash driver for XYZ123
    Add new flash driver for internal flash of ...
    target/cortex_m: fix segmentation fault in cmd 'soft_reset_halt'
    soft_reset_halt command failed reproducibly under following conditions: ...
    Test for NULL pointer and return error ...
    Reported-by: John Reporter <rep9876@gmail.com>
    Fixes: 123456789abc ("target: the commit where the problem started")
    BugLink: https://sourceforge.net/p/openocd/tickets/999/
    doc: fix typos
    See "git log" for more examples.
  6. Next you need to make sure that your patches are on top of the latest stuff on the server and that there are no conflicts:
    git pull --rebase origin master
  7. Send the patches to the Gerrit server for review:
    git push review
  8. Forgot something, want to add more? Just make the changes and do:
    git commit --amend
    git push review

Further reading: http://www.coreboot.org/Git

About checkpatch script

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.

Attention
The procedure below is allowed only for exceptional cases. Do not use it to submit normal patches.

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:

CHECK:CAMELCASE: Avoid CamelCase: <WSAGetLastError>
#96105: FILE: src/helper/log.c:505:
+ error_code = WSAGetLastError();

Adding in the commit message of the patch the line:

Checkpatch-ignore: CAMELCASE

will force checkpatch to ignore the CAMELCASE error.

When can I expect my contribution to be committed?

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:

  • the concerns raised in the review have been addressed (or explained),
  • reviewer does not re-examine the change in a month,
  • reviewer does not answer e-mails for another month.

Browsing Patches

All OpenOCD patches can be reviewed here.

Reviewing Patches

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.