mirror of
https://git.code.sf.net/p/openocd/code
synced 2024-11-21 16:15:31 +00:00
d4a64e3f38
Add support for code coverage collection. This helps developers to check if their test scenarios really exercised all the OpenOCD functionality that they intended to test. - Option --enable-gcov has been added to configure.ac which enables the coverage collection using Gcov. (Disabled by default.) - The steps to collect and inspect the coverage have been described in HACKING file. Change-Id: I259e401937a255e7ad7f155359a0b7787e4d0752 Signed-off-by: Jan Matyas <jan.matyas@codasip.com> Reviewed-on: https://review.openocd.org/c/openocd/+/8521 Tested-by: jenkins Reviewed-by: Evgeniy Naydanov <evgeniy.naydanov@syntacore.com> Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
366 lines
14 KiB
Plaintext
366 lines
14 KiB
Plaintext
// This file is part of the Doxygen Developer Manual
|
|
/** @page patchguide 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.
|
|
|
|
\attention 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 @a Settings, select @a
|
|
Identities pane, press <em>Link Another Identity</em> button. In case
|
|
you already have duplicated accounts, ask administrators for manual
|
|
merging.
|
|
|
|
\attention If you're behind a corporate wall with http only access to the
|
|
world, you can still use these instructions!
|
|
|
|
@section gerrit 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
|
|
@c 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:
|
|
@code
|
|
mkdir build-scanbuild; cd build-scanbuild
|
|
scan-build ../configure
|
|
scan-build make CFLAGS="-std=gnu99 -I. -I../../jimtcl"
|
|
@endcode
|
|
|
|
- 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 @c -fsanitize=* command arguments.
|
|
|
|
Example usage:
|
|
@code
|
|
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
|
|
@endcode
|
|
|
|
- Sparse Static Analyzer
|
|
|
|
Using this tool allows identifying some bug in C code.
|
|
In the future, OpenOCD would use the sparse attribute 'bitwise' to
|
|
detect incorrect endianness assignments.
|
|
|
|
Example usage:
|
|
@code
|
|
mkdir build-sparse; cd build-sparse
|
|
../configure CC=cgcc CFLAGS="-Wsparse-all -Wno-declaration-after-statement \
|
|
-Wno-unknown-attribute -Wno-transparent-union -Wno-tautological-compare \
|
|
-Wno-vla -Wno-flexible-array-array -D__FLT_EVAL_METHOD__=0"
|
|
make
|
|
@endcode
|
|
|
|
- Code coverage analysis
|
|
|
|
By inspecting the code coverage, you can identify potential gaps in your testing
|
|
and use that information to improve your test scenarios.
|
|
|
|
Example usage:
|
|
@code
|
|
mkdir build-gcov; cd build-gcov
|
|
../configure --enable-gcov [...]
|
|
make
|
|
# ... Now execute your test scenarios to collect OpenOCD code coverage ...
|
|
lcov --capture --directory ./src --output-file openocd-coverage.info
|
|
genhtml openocd-coverage.info --output-directory coverage_report
|
|
# ... Open coverage_report/index.html in a web browser ...
|
|
@endcode
|
|
|
|
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.
|
|
|
|
@section stepbystep Step by step procedure
|
|
|
|
-# Create a Gerrit account at: https://review.openocd.org
|
|
- On subsequent sign ins, use the full URL prefaced with 'http://'
|
|
For example: http://user_identifier.open_id_provider.com
|
|
-# 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.
|
|
-# 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.
|
|
-# 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.
|
|
-# Clone the git repository, rather than just download the source:
|
|
@code
|
|
git clone git://git.code.sf.net/p/openocd/code openocd
|
|
@endcode
|
|
or if you have problems with the "git:" protocol, use
|
|
the slower http protocol:
|
|
@code
|
|
git clone http://git.code.sf.net/p/openocd/code openocd
|
|
@endcode
|
|
-# Set up Gerrit with your local repository. All this does it
|
|
to instruct git locally how to send off the changes.
|
|
-# Add a new remote to git using Gerrit username:
|
|
@code
|
|
git remote add review ssh://USERNAME@review.openocd.org:29418/openocd.git
|
|
git config remote.review.push HEAD:refs/for/master
|
|
@endcode
|
|
Or with http only:
|
|
@code
|
|
git remote add review https://USERNAME@review.openocd.org/p/openocd.git
|
|
git config remote.review.push HEAD:refs/for/master
|
|
@endcode
|
|
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:
|
|
@code
|
|
git remote add review https://USERNAME:PASSWORD@review.openocd.org/p/openocd.git
|
|
@endcode
|
|
\note All contributions should be pushed to @c refs/for/master on the
|
|
Gerrit server, even if you plan to use several local branches for different
|
|
topics. It is possible because @c for/master is not a traditional Git
|
|
branch.
|
|
-# You will need to install this hook, we will look into a better solution:
|
|
@code
|
|
wget https://review.openocd.org/tools/hooks/commit-msg
|
|
mv commit-msg .git/hooks
|
|
chmod +x .git/hooks/commit-msg
|
|
@endcode
|
|
\note A script exists to simplify the two items above. Execute:
|
|
@code
|
|
tools/initial.sh <username>
|
|
@endcode
|
|
With @<username@> being your Gerrit username.
|
|
-# Set up git with your name and email:
|
|
@code
|
|
git config --global user.name "John Smith"
|
|
git config --global user.email "john@smith.org"
|
|
@endcode
|
|
-# Work on your patches. Split the work into
|
|
multiple small patches that can be reviewed and
|
|
applied separately and safely to the OpenOCD
|
|
repository.
|
|
@code
|
|
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.
|
|
}
|
|
@endcode
|
|
\note use "git add ." before commit to add new files.
|
|
|
|
\note check @ref checkpatch for hint about checkpatch script
|
|
|
|
Commit message template, notice the short first line.
|
|
The field '<c>specify touched area</c>'
|
|
should identify the main part or subsystem the patch touches.
|
|
@code{.unparsed}
|
|
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: ...
|
|
@endcode
|
|
Examples:
|
|
@code{.unparsed}
|
|
flash/nor/atsame5: add SAME59 support
|
|
|
|
Add new device ID
|
|
@endcode
|
|
@code{.unparsed}
|
|
flash/nor: flash driver for XYZ123
|
|
|
|
Add new flash driver for internal flash of ...
|
|
@endcode
|
|
@code{.unparsed}
|
|
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/
|
|
@endcode
|
|
@code{.unparsed}
|
|
doc: fix typos
|
|
@endcode
|
|
See "git log" for more examples.
|
|
|
|
-# 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:
|
|
@code
|
|
git pull --rebase origin master
|
|
@endcode
|
|
-# Send the patches to the Gerrit server for review:
|
|
@code
|
|
git push review
|
|
@endcode
|
|
-# Forgot something, want to add more? Just make the changes and do:
|
|
@code
|
|
git commit --amend
|
|
git push review
|
|
@endcode
|
|
|
|
Further reading: http://www.coreboot.org/Git
|
|
|
|
@section checkpatch About checkpatch script
|
|
|
|
OpenOCD source code includes the script checkpatch to let developers to
|
|
verify their patches before submitting them for review (see @ref gerrit).
|
|
|
|
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 <em>false
|
|
positive</em>!
|
|
|
|
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 <em>exceptional
|
|
cases</em>. Do not use it to submit normal patches.
|
|
|
|
There are <em>exceptional cases</em> 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 <em>exceptional cases</em>, 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.
|
|
For errors in the commit message, the special field has to be put in
|
|
the commit message before the line that produces the error.
|
|
The special field must be added <em>before</em> the 'Signed-off-by:'
|
|
line, otherwise it is ignored.
|
|
To ignore multiple errors, either add multiple lines with the special
|
|
field or add multiple error types, separated by space or commas, in a
|
|
single line.
|
|
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:
|
|
@code
|
|
CHECK:CAMELCASE: Avoid CamelCase: <WSAGetLastError>
|
|
#96105: FILE: src/helper/log.c:505:
|
|
+ error_code = WSAGetLastError();
|
|
@endcode
|
|
Adding in the commit message of the patch the line:
|
|
@code
|
|
Checkpatch-ignore: CAMELCASE
|
|
@endcode
|
|
will force checkpatch to ignore the CAMELCASE error.
|
|
|
|
@section timeline 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.
|
|
|
|
@section browsing Browsing Patches
|
|
All OpenOCD patches can be reviewed <a href="https://review.openocd.org/">here</a>.
|
|
|
|
@section reviewing Reviewing Patches
|
|
From the main <a href="https://review.openocd.org/#/q/status:open,n,z">Review
|
|
page</a> 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.
|
|
*/
|
|
/** @file
|
|
This file contains the @ref patchguide page.
|
|
*/
|