HACKING.txt

   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.