- 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 be easily reviewable. Don't make
+issue, and make it easily reviewable. Don't make
it so large that it's hard to review; split large
-patches into smaller ones. (That can also help
-track down bugs later on.) All patches should
+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.
+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
+
+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
@section stepbystep Step by step procedure
--# Create a Gerrit account at: http://openocd.zylin.com
+-# 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.
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@openocd.zylin.com:29418/openocd.git
-git config remote.review.push HEAD:refs/publish/master
+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 http://USERNAME@openocd.zylin.com/p/openocd.git
-git config remote.review.push HEAD:refs/publish/master
+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 - http://openocd.zylin.com/#/settings/http-password.
+ 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 http://USERNAME:PASSWORD@openocd.zylin.com/p/openocd.git
+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
-scp -p -P 29418 USERNAME@openocd.zylin.com:hooks/commit-msg .git/hooks/
+scp -p -P 29418 USERNAME@review.openocd.org:hooks/commit-msg .git/hooks/
@endcode
Or with http only:
@code
-wget http://openocd.zylin.com/tools/hooks/commit-msg
+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:
+ \note A script exists to simplify the two items above. Execute:
@code
tools/initial.sh <username>
@endcode
@endcode
-# Work on your patches. Split the work into
multiple small patches that can be reviewed and
- applied seperately and safely to the OpenOCD
+ applied separately and safely to the OpenOCD
repository.
@code
while(!done) {
@endcode
\note use "git add ." before commit to add new files.
- Comment template, notice the short first line w/topic. The topic field
- should identify the main part or subsystem the patch touches. Check
- git log for examples.
-@code
-topic: Short comment
+ 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,
<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:
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 oportunity to perform a review and raise objections.
+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
- reviewer does not answer e-mails for another month.
@section browsing Browsing Patches
-All OpenOCD patches can be reviewed <a href="http://openocd.zylin.com/">here</a>.
+All OpenOCD patches can be reviewed <a href="https://review.openocd.org/">here</a>.
@section reviewing Reviewing Patches
-From the main <a href="http://openocd.zylin.com/#/q/status:open,n,z">Review
+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"