]>
Commit | Line | Data |
---|---|---|
1 | "Coding styles are like assholes, everyone has one and no one likes anyone elses." | |
2 | --Eric Warmenhoven | |
3 | ||
4 | The Proxmark3 codebase is pretty messy and in the process of being cleaned up, | |
5 | so we don't have clear guidelines on how to place new code just yet. However, | |
6 | please don't make things worse. | |
7 | ||
8 | However, we have established a set of coding style guidelines in order to | |
9 | clean up the code consistently and keep it consistent in the future. Use common | |
10 | sense and good taste. If breaking a rule leads to cleaner code, you can do so, | |
11 | but laziness is not an excuse. | |
12 | ||
13 | === INDENTATION === | |
14 | ||
15 | Use tabs for indentation, but use spaces for alignment: | |
16 | ||
17 | if (foo(this, that, there) | |
18 | && bar == baz) | |
19 | { | |
20 | dostuff(); | |
21 | } | |
22 | ||
23 | Notice it's like this (T___ for tab, S for space, for a 4-char tab setting): | |
24 | ||
25 | T___if (foo(this, that, there) | |
26 | T___SSSS&& bar == baz) | |
27 | ||
28 | Another example: | |
29 | ||
30 | #define THIS 0x10 | |
31 | #define THAT_THING 0x20 | |
32 | #define SOMETHING_ELSE 0x80 | |
33 | ||
34 | These should look good no matter what your editor's tab setting is, so go nuts | |
35 | and pick whatever you like best. | |
36 | ||
37 | === WIDTH === | |
38 | ||
39 | Try to keep lines to a reasonable length. 80 characters is a good mark; using an | |
40 | editor that shows a vertical line is a great idea. However, don't break a line | |
41 | just because you're slightly over, it's not worth it. No 200-character lines, | |
42 | though. | |
43 | ||
44 | === MACROS === | |
45 | ||
46 | #defines, function-like or not, are all UPPERCASE unless you're emulating a | |
47 | well-known function name. | |
48 | ||
49 | === IDENTIFIERS === | |
50 | ||
51 | Functions, local variables, and arguments are all named using | |
52 | underscores_as_spaces. Global variables are Evil and are prepended with g_ to | |
53 | distinguish them. Avoid them. | |
54 | ||
55 | Single-character variables are a bad idea. Exceptions: loop iterators and maybe | |
56 | simple byte pointers (*p) in very obvious places. If you have more than one | |
57 | such pointer, use a real name. If you have more than a couple nested loops, | |
58 | complex logic, or indices that differ in interpretation or purpose, use real | |
59 | names instead of i,j,k. | |
60 | ||
61 | === DATA TYPES === | |
62 | ||
63 | Use stdint.h types (uint32_t and friends) unless you have a reason not to. Don't | |
64 | use microsoft-style DWORD and the like, we're getting rid of those. Avoid char | |
65 | for buffers, uint8_t is more obvious when you're not working with strings. Use | |
66 | 'const' where things are const. Try to use size_t for sizes. | |
67 | ||
68 | Pointers are: | |
69 | void *ptr; | |
70 | not: | |
71 | void* ptr; | |
72 | otherwise you're tempted to write: | |
73 | void* in, out; | |
74 | and you'll fail. | |
75 | ||
76 | === EXPRESSIONS === | |
77 | ||
78 | In general, use whitespace around binary operators - no unspaced blobs of an | |
79 | expression. This rule may be broken if it makes things clearer. For example, | |
80 | ||
81 | if (5*a < b && some_bool_var) | |
82 | ||
83 | but not | |
84 | ||
85 | if (5*a<b&&some_bool_var) | |
86 | ||
87 | For equality with constants, use i == 0xF00, not 0xF00 == i. The compiler warns | |
88 | you about = vs == anyway, and you shouldn't be screwing that one up by now | |
89 | anyway. | |
90 | ||
91 | === IF / FOR / WHILE / etc. === | |
92 | ||
93 | Put the opening brace on the same line, with a space before it. Exception: if | |
94 | the if/for/while condition/whatever are split over several lines, it might be | |
95 | more appealing to put the opening brace on its own line, so use your own | |
96 | judgement there: | |
97 | ||
98 | if (foo(this, that, there) | |
99 | && bar == baz) | |
100 | { | |
101 | dostuff(); | |
102 | } | |
103 | ||
104 | If you do split the condition, put the binary operators that join the lines at | |
105 | the beginning of the following lines (as above), not at the end of the prior | |
106 | lines. | |
107 | ||
108 | There should be a space between the construct name (if/for/whatever) and the | |
109 | opening parenthesis, and there should be a space between the closing parenthesis | |
110 | and the opening brace. | |
111 | ||
112 | For generic for() iterator variables, declare them in-line: | |
113 | ||
114 | for (int i = 0; i < 10; i++) { | |
115 | ... | |
116 | } | |
117 | ||
118 | Note the spaces after the semicolons. | |
119 | ||
120 | if/else should be laid out as follows: | |
121 | ||
122 | if (foo) { | |
123 | ... | |
124 | } else if (bar) { | |
125 | ... | |
126 | } else { | |
127 | ... | |
128 | } | |
129 | ||
130 | or | |
131 | ||
132 | if (foo) | |
133 | ... | |
134 | else if (bar) | |
135 | ... | |
136 | else | |
137 | ... | |
138 | ||
139 | Don't mix braces vs. no braces. If any of your bodies are > 1 line, put braces | |
140 | around them all. | |
141 | ||
142 | === FUNCTIONS === | |
143 | ||
144 | Functions with no arguments are declared as f(void), not f(). Put the return | |
145 | type on the same line. Use static for functions that aren't exported, and put | |
146 | exported functions in a header file (one header file per source file with | |
147 | exported functions usually, no huge headers with all functions). Put a space | |
148 | after a comma in argument lists. | |
149 | ||
150 | void foo(int a_thing, int something_else) | |
151 | { | |
152 | ... | |
153 | } | |
154 | ||
155 | void baz(void) | |
156 | { | |
157 | foo(bluh, blah); | |
158 | } | |
159 | ||
160 | Function names should be separated_with_underscores(), except for standard | |
161 | functions (memcpy, etc.). It may make sense to break this rule for very common, | |
162 | generic functions that look like library functions (e.g. dprintf()). | |
163 | ||
164 | Don't use single-character arguments. Exception: very short functions with one | |
165 | argument that's really obvious: | |
166 | ||
167 | static int ascii(char c) | |
168 | { | |
169 | if (c < 0x20 || c >= 0x7f) | |
170 | return '.'; | |
171 | else | |
172 | return c; | |
173 | } | |
174 | ||
175 | vs. | |
176 | ||
177 | static void hexdump(void *buf, size_t len) | |
178 | { | |
179 | ... | |
180 | } | |
181 | ||
182 | As a general guideline, functions shouldn't usually be much more than 30-50 | |
183 | lines. Above, the general algorithm won't be easily apparent, and you're | |
184 | probably missing some factoring/restructuring opportunity. | |
185 | ||
186 | === STRUCTS / UNIONS / ENUMS === | |
187 | ||
188 | Use typedefs when defining structs. The type should be named something_t. | |
189 | ||
190 | typedef struct { | |
191 | blah blah; | |
192 | } prox_cmd_t; | |
193 | ||
194 | You can use anonymous enums to replace lots of sequential or mostly-sequential | |
195 | #defines. | |
196 | ||
197 | === SWITCH === | |
198 | ||
199 | Indent once for the case: labels, then again for the body. Like this: | |
200 | ||
201 | switch(bar) { | |
202 | case OPTION_A: | |
203 | do_stuff(); | |
204 | break; | |
205 | case OPTION_B: | |
206 | do_other_stuff(); | |
207 | break; | |
208 | } | |
209 | ||
210 | If you fall through into another case, add an explicit comment; otherwise, it | |
211 | can look confusing. | |
212 | ||
213 | If your switch() is too long or has too many cases, it should be cleaned up. | |
214 | Split off the cases into functions, break the switch() into parent and children | |
215 | switches (e.g. command and subcommand), or use an array of function pointers or | |
216 | the like. In other words, use common sense and your brain. | |
217 | ||
218 | If you need local scope variables for a case, you can add braces: | |
219 | ||
220 | switch(bar) { | |
221 | case OPTION_A: { | |
222 | int baz = 5*bar; | |
223 | do_stuff(baz); | |
224 | break; | |
225 | } | |
226 | ... | |
227 | ||
228 | But at that point you should probably consider using a separate function. | |
229 | ||
230 | === COMMENTS === | |
231 | ||
232 | Use //, it's shorter: | |
233 | ||
234 | // this does foo | |
235 | ... | |
236 | ||
237 | // baz: | |
238 | // This does blah blah blah ..... | |
239 | // blah blah... | |
240 | ||
241 | /* */ can be used to comment blocks of code, but you should probably remove | |
242 | them anyway - we have version control, it's easy to fetch old code if needed, | |
243 | so avoid committing commented out chunks of code. The same goes for #if 0. | |
244 | ||
245 | === FILE === | |
246 | ||
247 | Please use common sense and restrain yourself from having a thousands+++ line | |
248 | file. Functions in a file should have something *specific* in common. Over time | |
249 | sub-categories can arise and should therefore yield to file splitting. | |
250 | ||
251 | For these reasons, vague and general filenames (e.g. util.*, global.*, misc.*, | |
252 | main.*, and the like) should be very limited, if not prohibited. | |
253 | ||
254 | === FILE HEADERS === | |
255 | ||
256 | License/description header first: | |
257 | ||
258 | //----------------------------------------------------------------------------- | |
259 | // YOUR COPYRIGHT LINE GOES HERE | |
260 | // | |
261 | // This code is licensed to you under the terms of the GNU GPL, version 2 or, | |
262 | // at your option, any later version. See the LICENSE.txt file for the text of | |
263 | // the license. | |
264 | //----------------------------------------------------------------------------- | |
265 | // FILE DESCRIPTION GOES HERE | |
266 | //----------------------------------------------------------------------------- | |
267 | ||
268 | If you modify a file in any non-trivial way (add code, etc.), add your copyright | |
269 | to the top. | |
270 | ||
271 | === HEADER FILES === | |
272 | ||
273 | Use the following include guard format: | |
274 | ||
275 | #ifndef FOOBAR_H__ | |
276 | #define FOOBAR_H__ | |
277 | ||
278 | ... | |
279 | ||
280 | #endif // FOOBAR_H__ | |
281 | ||
282 | Keep in mind that __FOOBAR_H would be reserved by the implementation and thus | |
283 | you shouldn't use it (same for _FOOBAR_H). | |
284 | ||
285 | === WHITESPACE === | |
286 | ||
287 | Avoid trailing whitespace (no line should end in tab or space). People forget | |
288 | this all the time if their editor doesn't handle it, but don't be surprised if | |
289 | you see someone fixing it from time to time. | |
290 | ||
291 | Keep a newline (blank line) at the end of each file. |