-
Notifications
You must be signed in to change notification settings - Fork 330
/
HACKING
375 lines (317 loc) · 14.3 KB
/
HACKING
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
// 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
- Runtime coverage testing
Apply the following patch to prevent OpenOCD from killing itself:
@code
--- a/src/openocd.c
+++ b/src/openocd.c
@@ -372,8 +372,6 @@ int openocd_main(int argc, char *argv[])
if (ERROR_FAIL == ret)
return EXIT_FAILURE;
- else if (ERROR_OK != ret)
- exit_on_signal(ret);
return ret;
}
@endcode
Configure your OpenOCD binary with coverage support as follows:
@code
LDFLAGS="-fprofile-arcs -ftest-coverage"
CFLAGS="-fprofile-arcs -ftest-coverage" ./configure
@endcode
Now every time OpenOCD is run, coverage info in your build directory is
updated. Running `gcov src/path/file.c` will generate a report.
- 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
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.
*/