6620e46b |
1 | "Coding styles are like assholes, everyone has one and no one likes anyone elses." |
4fc25350 |
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 | |
6620e46b |
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 | |
4fc25350 |
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 | |
6620e46b |
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 | |
4fc25350 |
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. |