OpenOCD
HACKING
Go to the documentation of this file.
1 // This file is part of the Doxygen Developer Manual
2 /** @page patchguide Patch Guidelines
3 
4 \attention You can't send patches to the mailing list anymore at all. Nowadays
5 you are expected to send patches to the OpenOCD Gerrit GIT server for a
6 review.
7 
8 \attention If you already have a Gerrit account and want to try a
9 different sign in method, please first sign in as usually, press your
10 name in the upper-right corner, go to @a Settings, select @a
11 Identities pane, press <em>Link Another Identity</em> button. In case
12 you already have duplicated accounts, ask administrators for manual
13 merging.
14 
15 \attention If you're behind a corporate wall with http only access to the
16 world, you can still use these instructions!
17 
18 @section gerrit Submitting patches to the OpenOCD Gerrit server
19 
20 OpenOCD is to some extent a "self service" open source project, so to
21 contribute, you must follow the standard procedures to have the best
22 possible chance to get your changes accepted.
23 
24 The procedure to create a patch is essentially:
25 
26 - make the changes
27 - create a commit
28 - send the changes to the Gerrit server for review
29 - correct the patch and re-send it according to review feedback
30 
31 Your patch (or commit) should be a "good patch": focus it on a single
32 issue, and make it easily reviewable. Don't make
33 it so large that it's hard to review; split large
34 patches into smaller ones (this will also help
35 to track down bugs later). All patches should
36 be "clean", which includes preserving the existing
37 coding style and updating documentation as needed. When adding a new
38 command, the corresponding documentation should be added to
39 @c doc/openocd.texi in the same commit. OpenOCD runs on both Little
40 Endian and Big Endian hosts so the code can't count on specific byte
41 ordering (in other words, must be endian-clean).
42 
43 There are several additional methods of improving the quality of your
44 patch:
45 
46 - Runtime testing with Valgrind Memcheck
47 
48  This helps to spot memory leaks, undefined behaviour due to
49  uninitialized data or wrong indexing, memory corruption, etc.
50 
51 - Clang Static Analyzer
52 
53  Using this tool uncovers many different kinds of bugs in C code,
54  with problematic execution paths fully explained. It is a part of
55  standard Clang installation.
56 
57  To generate a report, run this in the OpenOCD source directory:
58  @code
59  mkdir build-scanbuild; cd build-scanbuild
60  scan-build ../configure
61  scan-build make CFLAGS="-std=gnu99 -I. -I../../jimtcl"
62  @endcode
63 
64 - Runtime testing with sanitizers
65 
66  Both GCC and LLVM/Clang include advanced instrumentation options to
67  detect undefined behaviour and many kinds of memory
68  errors. Available with @c -fsanitize=* command arguments.
69 
70  Example usage:
71  @code
72  mkdir build-sanitizers; cd build-sanitizers
73  ../configure CC=clang CFLAGS="-fno-omit-frame-pointer \
74  -fsanitize=address -fsanitize=undefined -ggdb3"
75  make
76  export ASAN_OPTIONS=detect_stack_use_after_return=1
77  src/openocd -s ../tcl -f /path/to/openocd.cfg
78  @endcode
79 
80 - Sparse Static Analyzer
81 
82  Using this tool allows identifying some bug in C code.
83  In the future, OpenOCD would use the sparse attribute 'bitwise' to
84  detect incorrect endianness assignments.
85 
86  Example usage:
87  @code
88  mkdir build-sparse; cd build-sparse
89  ../configure CC=cgcc CFLAGS="-Wsparse-all -Wno-declaration-after-statement \
90  -Wno-unknown-attribute -Wno-transparent-union -Wno-tautological-compare \
91  -Wno-vla -Wno-flexible-array-array -D__FLT_EVAL_METHOD__=0"
92  make
93  @endcode
94 
95 - Code coverage analysis
96 
97  By inspecting the code coverage, you can identify potential gaps in your testing
98  and use that information to improve your test scenarios.
99 
100  Example usage:
101  @code
102  mkdir build-gcov; cd build-gcov
103  ../configure --enable-gcov [...]
104  make
105  # ... Now execute your test scenarios to collect OpenOCD code coverage ...
106  lcov --capture --directory ./src --output-file openocd-coverage.info
107  genhtml openocd-coverage.info --output-directory coverage_report
108  # ... Open coverage_report/index.html in a web browser ...
109  @endcode
110 
111 Please consider performing these additional checks where appropriate
112 (especially Clang Static Analyzer for big portions of new code) and
113 mention the results (e.g. "Valgrind-clean, no new Clang analyzer
114 warnings") in the commit message.
115 
116 Say in the commit message if it's a bugfix (describe the bug) or a new
117 feature. Don't expect patches to merge immediately
118 for the next release. Be ready to rework patches
119 in response to feedback.
120 
121 Add yourself to the GPL copyright for non-trivial changes.
122 
123 @section stepbystep Step by step procedure
124 
125 -# Create a Gerrit account at: https://review.openocd.org
126  - On subsequent sign ins, use the full URL prefaced with 'http://'
127  For example: http://user_identifier.open_id_provider.com
128  -# Add a username to your profile.
129  After creating the Gerrit account and signing in, you will need to
130  add a username to your profile. To do this, go to 'Settings', and
131  add a username of your choice.
132  Your username will be required in step 3 and substituted wherever
133  the string 'USERNAME' is found.
134  -# Create an SSH public key following the directions on github:
135  https://help.github.com/articles/generating-ssh-keys . You can skip step 3
136  (adding key to Github account) and 4 (testing) - these are useful only if
137  you actually use Github or want to test whether the new key works fine.
138  -# Add this new SSH key to your Gerrit account:
139  go to 'Settings' > 'SSH Public Keys', paste the contents of
140  ~/.ssh/id_rsa.pub into the text field (if it's not visible click on
141  'Add Key ...' button) and confirm by clicking 'Add' button.
142 -# Clone the git repository, rather than just download the source:
143  @code
144  git clone git://git.code.sf.net/p/openocd/code openocd
145  @endcode
146  or if you have problems with the "git:" protocol, use
147  the slower http protocol:
148  @code
149  git clone http://git.code.sf.net/p/openocd/code openocd
150  @endcode
151 -# Set up Gerrit with your local repository. All this does it
152 to instruct git locally how to send off the changes.
153  -# Add a new remote to git using Gerrit username:
154 @code
155 git remote add review ssh://USERNAME@review.openocd.org:29418/openocd.git
156 git config remote.review.push HEAD:refs/for/master
157 @endcode
158  Or with http only:
159 @code
160 git remote add review https://USERNAME@review.openocd.org/p/openocd.git
161 git config remote.review.push HEAD:refs/for/master
162 @endcode
163  The http password is configured from your gerrit settings - https://review.openocd.org/#/settings/http-password.
164  \note If you want to simplify http access you can also add your http password to the url as follows:
165 @code
166 git remote add review https://USERNAME:PASSWORD@review.openocd.org/p/openocd.git
167 @endcode
168  \note All contributions should be pushed to @c refs/for/master on the
169 Gerrit server, even if you plan to use several local branches for different
170 topics. It is possible because @c for/master is not a traditional Git
171 branch.
172  -# You will need to install this hook to automatically add the
173  field "Change-Id:" in the commit message, as required by Gerrit.
174  We will look into a better solution:
175 @code
176 wget https://review.openocd.org/tools/hooks/commit-msg
177 mv commit-msg .git/hooks
178 chmod +x .git/hooks/commit-msg
179 @endcode
180  \note A script exists to simplify the two items above. Execute:
181 @code
182 tools/initial.sh <username>
183 @endcode
184 With @<username@> being your Gerrit username.
185 -# Set up git with your name and email:
186 @code
187 git config --global user.name "John Smith"
188 git config --global user.email "john@smith.org"
189 @endcode
190 -# Work on your patches. Split the work into
191  multiple small patches that can be reviewed and
192  applied separately and safely to the OpenOCD
193  repository.
194 @code
195 while(!done) {
196  work - edit files using your favorite editor.
197  run "git commit -s -a" to commit all changes.
198  run tools/checkpatch.sh to verify your patch style is ok.
199 }
200 @endcode
201  \note use "git add ." before commit to add new files.
202 
203  \note check @ref checkpatch for hint about checkpatch script
204 
205  Commit message template, notice the short first line.
206  The field '<c>specify touched area</c>'
207  should identify the main part or subsystem the patch touches.
208 @code{.unparsed}
209 specify touched area: short comment
210 <blank line>
211 Longer comments over several lines, explaining (where applicable) the
212 reason for the patch and the general idea the solution is based on,
213 any major design decisions, etc. Limit each comment line's length to 75
214 characters; since 75 it's too short for a URL, you can put the URL in a
215 separate line preceded by 'Link: '.
216 <blank line>
217 Signed-off-by: ...
218 @endcode
219  Examples:
220 @code{.unparsed}
221 flash/nor/atsame5: add SAME59 support
222 
223 Add new device ID
224 @endcode
225 @code{.unparsed}
226 flash/nor: flash driver for XYZ123
227 
228 Add new flash driver for internal flash of ...
229 @endcode
230 @code{.unparsed}
231 target/cortex_m: fix segmentation fault in cmd 'soft_reset_halt'
232 
233 soft_reset_halt command failed reproducibly under following conditions: ...
234 Test for NULL pointer and return error ...
235 
236 Reported-by: John Reporter <rep9876@gmail.com>
237 Fixes: 123456789abc ("target: the commit where the problem started")
238 BugLink: https://sourceforge.net/p/openocd/tickets/999/
239 @endcode
240 @code{.unparsed}
241 doc: fix typos
242 @endcode
243  See "git log" for more examples.
244 
245 -# Next you need to make sure that your patches
246  are on top of the latest stuff on the server and
247  that there are no conflicts:
248 @code
249 git pull --rebase origin master
250 @endcode
251 
252 -# When you create a new version of an old patch, check that the new patch
253  keeps the same 'Change-Id:' field of the old patch.
254  This allows the Gerrit server to recognize the patch as a new version of
255  the older one and keeps track of the history and the review process.
256 
257 -# Send the patches to the Gerrit server for review:
258 @code
259 git push review
260 @endcode
261 -# Forgot something, want to add more? Just make the changes and do:
262 @code
263 git commit --amend
264 git push review
265 @endcode
266 
267 Further reading: http://www.coreboot.org/Git
268 
269 @section checkpatch About checkpatch script
270 
271 OpenOCD source code includes the script checkpatch to let developers to
272 verify their patches before submitting them for review (see @ref gerrit).
273 
274 Every patch for OpenOCD project that is submitted for review on Gerrit
275 is tested by Jenkins. Jenkins will run the checkpatch script to analyze
276 each patch.
277 If the script highlights either errors or warnings, Gerrit will add the
278 score "-1" to the patch and maintainers will probably ignore the patch,
279 waiting for the developer to send a fixed version.
280 
281 The script checkpatch verifies the SPDX tag for new files against a very
282 short list of license tags.
283 If the license of your contribution is not listed there, but compatible
284 with OpenOCD license, please alert the maintainers or add the missing
285 license in the first patch of your patch series.
286 
287 The script checkpatch has been originally developed for the Linux kernel
288 source code, thus includes specific tests and checks related to Linux
289 coding style and to Linux code structure. While the script has been
290 adapted for OpenOCD specificities, it still includes some Linux related
291 test. It is then possible that it triggers sometimes some <em>false
292 positive</em>!
293 
294 If you think that the error identified by checkpatch is a false
295 positive, please report it to the openocd-devel mailing list or prepare
296 a patch for fixing checkpatch and send it to Gerrit for review.
297 
298 \attention The procedure below is allowed only for <em>exceptional
299 cases</em>. Do not use it to submit normal patches.
300 
301 There are <em>exceptional cases</em> in which you need to skip some of
302 the tests from checkpatch in order to pass the approval from Gerrit.
303 
304 For example, a patch that modify one line inside a big comment block
305 will not show the beginning or the end of the comment block. This can
306 prevent checkpatch to detect the comment block. Checkpatch can wrongly
307 consider the modified comment line as a code line, triggering a set of
308 false errors.
309 
310 Only for <em>exceptional cases</em>, it is allowed to submit patches
311 to Gerrit with the special field 'Checkpatch-ignore:' in the commit
312 message. This field will cause checkpatch to ignore the error types
313 listed in the field, only for the patch itself.
314 For errors in the commit message, the special field has to be put in
315 the commit message before the line that produces the error.
316 The special field must be added <em>before</em> the 'Signed-off-by:'
317 line, otherwise it is ignored.
318 To ignore multiple errors, either add multiple lines with the special
319 field or add multiple error types, separated by space or commas, in a
320 single line.
321 The error type is printed by checkpatch on failure.
322 For example the names of Windows APIs mix lower and upper case chars,
323 in violation of OpenOCD coding style, triggering a 'CAMELCASE' error:
324 @code
325 CHECK:CAMELCASE: Avoid CamelCase: <WSAGetLastError>
326 #96105: FILE: src/helper/log.c:505:
327 + error_code = WSAGetLastError();
328 @endcode
329 Adding in the commit message of the patch the line:
330 @code
331 Checkpatch-ignore: CAMELCASE
332 @endcode
333 will force checkpatch to ignore the CAMELCASE error.
334 
335 @section timeline When can I expect my contribution to be committed?
336 
337 The code review is intended to take as long as a week or two to allow
338 maintainers and contributors who work on OpenOCD only in their spare
339 time opportunity to perform a review and raise objections.
340 
341 With Gerrit much of the urgency of getting things committed has been
342 removed as the work in progress is safely stored in Gerrit and
343 available if someone needs to build on your work before it is
344 submitted to the official repository.
345 
346 Another factor that contributes to the desire for longer cool-off
347 times (the time a patch lies around without any further changes or
348 comments), it means that the chances of quality regression on the
349 master branch will be much reduced.
350 
351 If a contributor pushes a patch, it is considered good form if another
352 contributor actually approves and submits that patch.
353 
354 It should be noted that a negative review in Gerrit ("-1" or "-2") may (but does
355 not have to) be disregarded if all conditions listed below are met:
356 
357 - the concerns raised in the review have been addressed (or explained),
358 - reviewer does not re-examine the change in a month,
359 - reviewer does not answer e-mails for another month.
360 
361 @section browsing Browsing Patches
362 All OpenOCD patches can be reviewed <a href="https://review.openocd.org/">here</a>.
363 
364 @section reviewing Reviewing Patches
365 From the main <a href="https://review.openocd.org/#/q/status:open,n,z">Review
366 page</a> select the patch you want to review and click on that patch. On the
367 appearing page select the download method (top right). Apply the
368 patch. After building and testing you can leave a note with the "Reply"
369 button and mark the patch with -1, 0 and +1.
370 */
371 /** @file
372 This file contains the @ref patchguide page.
373 */