Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 1 | Busybox Style Guide |
| 2 | =================== |
| 3 | |
| 4 | This document describes the coding style conventions used in Busybox. If you |
| 5 | add a new file to Busybox or are editing an existing file, please format your |
| 6 | code according to this style. If you are the maintainer of a file that does |
| 7 | not follow these guidelines, please -- at your own convenience -- modify the |
| 8 | file(s) you maintain to bring them into conformance with this style guide. |
| 9 | Please note that this is a low priority task. |
| 10 | |
| 11 | To help you format the whitespace of your programs, an ".indent.pro" file is |
| 12 | included in the main Busybox source directory that contains option flags to |
| 13 | format code as per this style guide. This way you can run GNU indent on your |
| 14 | files by typing 'indent myfile.c myfile.h' and it will magically apply all the |
| 15 | right formatting rules to your file. Please _do_not_ run this on all the files |
| 16 | in the directory, just your own. |
| 17 | |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 18 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 19 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 20 | Declaration Order |
| 21 | ----------------- |
| 22 | |
| 23 | Here is the order in which code should be laid out in a file: |
| 24 | |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 25 | - commented program name and one-line description |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 26 | - commented author name and email address(es) |
| 27 | - commented GPL boilerplate |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 28 | - commented longer description / notes for the program (if needed) |
Mark Whitley | 9ead689 | 2001-03-03 00:44:55 +0000 | [diff] [blame] | 29 | - #includes of .h files with angle brackets (<>) around them |
| 30 | - #includes of .h files with quotes ("") around them |
| 31 | - #defines (if any, note the section below titled "Avoid the Preprocessor") |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 32 | - const and global variables |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 33 | - function declarations (if necessary) |
| 34 | - function implementations |
| 35 | |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 36 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 37 | |
| 38 | Whitespace and Formatting |
| 39 | ------------------------- |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 40 | |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 41 | This is everybody's favorite flame topic so let's get it out of the way right |
| 42 | up front. |
| 43 | |
| 44 | |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 45 | Tabs vs. Spaces in Line Indentation |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 46 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 47 | |
| 48 | The preference in Busybox is to indent lines with tabs. Do not indent lines |
| 49 | with spaces and do not indents lines using a mixture of tabs and spaces. (The |
| 50 | indentation style in the Apache and Postfix source does this sort of thing: |
| 51 | \s\s\s\sif (expr) {\n\tstmt; --ick.) The only exception to this rule is |
| 52 | multi-line comments that use an asterisk at the beginning of each line, i.e.: |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 53 | |
Mike Frysinger | 434ccd9 | 2006-07-05 22:19:21 +0000 | [diff] [blame^] | 54 | \t/* |
| 55 | \t * This is a block comment. |
| 56 | \t * Note that it has multiple lines |
| 57 | \t * and that the beginning of each line has a tab plus a space |
| 58 | \t * except for the opening '/*' line where the slash |
| 59 | \t * is used instead of a space. |
| 60 | \t */ |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 61 | |
| 62 | Furthermore, The preference is that tabs be set to display at four spaces |
| 63 | wide, but the beauty of using only tabs (and not spaces) at the beginning of |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 64 | lines is that you can set your editor to display tabs at *whatever* number of |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 65 | spaces is desired and the code will still look fine. |
| 66 | |
| 67 | |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 68 | Operator Spacing |
| 69 | ~~~~~~~~~~~~~~~~ |
| 70 | |
| 71 | Put spaces between terms and operators. Example: |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 72 | |
| 73 | Don't do this: |
| 74 | |
| 75 | for(i=0;i<num_items;i++){ |
| 76 | |
| 77 | Do this instead: |
| 78 | |
| 79 | for (i = 0; i < num_items; i++) { |
| 80 | |
| 81 | While it extends the line a bit longer, the spaced version is more |
| 82 | readable. An allowable exception to this rule is the situation where |
| 83 | excluding the spacing makes it more obvious that we are dealing with a |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 84 | single term (even if it is a compound term) such as: |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 85 | |
| 86 | if (str[idx] == '/' && str[idx-1] != '\\') |
| 87 | |
| 88 | or |
| 89 | |
| 90 | if ((argc-1) - (optind+1) > 0) |
| 91 | |
| 92 | |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 93 | Bracket Spacing |
| 94 | ~~~~~~~~~~~~~~~ |
| 95 | |
| 96 | If an opening bracket starts a function, it should be on the |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 97 | next line with no spacing before it. However, if a bracket follows an opening |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 98 | control block, it should be on the same line with a single space (not a tab) |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 99 | between it and the opening control block statement. Examples: |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 100 | |
| 101 | Don't do this: |
| 102 | |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 103 | while (!done) |
| 104 | { |
| 105 | |
| 106 | do |
| 107 | { |
| 108 | |
| 109 | Don't do this either: |
| 110 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 111 | while (!done){ |
Mark Whitley | 925edb8 | 2001-02-03 00:20:14 +0000 | [diff] [blame] | 112 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 113 | do{ |
| 114 | |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 115 | And for heaven's sake, don't do this: |
| 116 | |
| 117 | while (!done) |
| 118 | { |
Mark Whitley | 925edb8 | 2001-02-03 00:20:14 +0000 | [diff] [blame] | 119 | |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 120 | do |
| 121 | { |
| 122 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 123 | Do this instead: |
| 124 | |
| 125 | while (!done) { |
Mark Whitley | 925edb8 | 2001-02-03 00:20:14 +0000 | [diff] [blame] | 126 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 127 | do { |
| 128 | |
Mike Frysinger | 0612b5f | 2006-02-24 01:18:24 +0000 | [diff] [blame] | 129 | Exceptions: |
| 130 | |
| 131 | - if you have long logic statements that need to be wrapped, then uncuddling |
| 132 | the bracket to improve readability is allowed: |
| 133 | |
| 134 | if (some_really_long_checks && some_other_really_long_checks \ |
| 135 | && some_more_really_long_checks) |
| 136 | { |
| 137 | do_foo_now; |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 138 | |
Mark Whitley | 925edb8 | 2001-02-03 00:20:14 +0000 | [diff] [blame] | 139 | Spacing around Parentheses |
| 140 | ~~~~~~~~~~~~~~~~~~~~~~~~~~ |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 141 | |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 142 | Put a space between C keywords and left parens, but not between function names |
| 143 | and the left paren that starts it's parameter list (whether it is being |
| 144 | declared or called). Examples: |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 145 | |
| 146 | Don't do this: |
| 147 | |
| 148 | while(foo) { |
| 149 | for(i = 0; i < n; i++) { |
| 150 | |
| 151 | Do this instead: |
| 152 | |
| 153 | while (foo) { |
| 154 | for (i = 0; i < n; i++) { |
| 155 | |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 156 | But do functions like this: |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 157 | |
| 158 | static int my_func(int foo, char bar) |
| 159 | ... |
| 160 | baz = my_func(1, 2); |
| 161 | |
Mark Whitley | 925edb8 | 2001-02-03 00:20:14 +0000 | [diff] [blame] | 162 | Also, don't put a space between the left paren and the first term, nor between |
| 163 | the last arg and the right paren. |
| 164 | |
| 165 | Don't do this: |
| 166 | |
| 167 | if ( x < 1 ) |
| 168 | strcmp( thisstr, thatstr ) |
| 169 | |
| 170 | Do this instead: |
| 171 | |
| 172 | if (x < 1) |
| 173 | strcmp(thisstr, thatstr) |
| 174 | |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 175 | |
| 176 | Cuddled Elses |
| 177 | ~~~~~~~~~~~~~ |
| 178 | |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 179 | Also, please "cuddle" your else statements by putting the else keyword on the |
| 180 | same line after the right bracket that closes an 'if' statement. |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 181 | |
| 182 | Don't do this: |
| 183 | |
| 184 | if (foo) { |
| 185 | stmt; |
| 186 | } |
| 187 | else { |
| 188 | stmt; |
| 189 | } |
| 190 | |
| 191 | Do this instead: |
| 192 | |
| 193 | if (foo) { |
| 194 | stmt; |
| 195 | } else { |
| 196 | stmt; |
| 197 | } |
| 198 | |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 199 | The exception to this rule is if you want to include a comment before the else |
| 200 | block. Example: |
| 201 | |
| 202 | if (foo) { |
| 203 | stmts... |
| 204 | } |
| 205 | /* otherwise, we're just kidding ourselves, so re-frob the input */ |
| 206 | else { |
| 207 | other_stmts... |
| 208 | } |
| 209 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 210 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 211 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 212 | Variable and Function Names |
| 213 | --------------------------- |
| 214 | |
| 215 | Use the K&R style with names in all lower-case and underscores occasionally |
Mark Whitley | 9028e2c | 2000-11-17 21:28:39 +0000 | [diff] [blame] | 216 | used to separate words (e.g., "variable_name" and "numchars" are both |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 217 | acceptable). Using underscores makes variable and function names more readable |
| 218 | because it looks like whitespace; using lower-case is easy on the eyes. |
| 219 | |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 220 | Frowned upon: |
| 221 | |
| 222 | hitList |
| 223 | TotalChars |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 224 | szFileName |
| 225 | pf_Nfol_TriState |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 226 | |
| 227 | Preferred: |
| 228 | |
| 229 | hit_list |
| 230 | total_chars |
| 231 | file_name |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 232 | sensible_name |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 233 | |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 234 | Exceptions: |
| 235 | |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 236 | - Enums, macros, and constant variables are occasionally written in all |
| 237 | upper-case with words optionally seperatedy by underscores (i.e. FIFOTYPE, |
| 238 | ISBLKDEV()). |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 239 | |
| 240 | - Nobody is going to get mad at you for using 'pvar' as the name of a |
| 241 | variable that is a pointer to 'var'. |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 242 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 243 | |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 244 | Converting to K&R |
| 245 | ~~~~~~~~~~~~~~~~~ |
| 246 | |
| 247 | The Busybox codebase is very much a mixture of code gathered from a variety of |
| 248 | sources. This explains why the current codebase contains such a hodge-podge of |
| 249 | different naming styles (Java, Pascal, K&R, just-plain-weird, etc.). The K&R |
| 250 | guideline explained above should therefore be used on new files that are added |
| 251 | to the repository. Furthermore, the maintainer of an existing file that uses |
| 252 | alternate naming conventions should, at his own convenience, convert those |
| 253 | names over to K&R style. Converting variable names is a very low priority |
| 254 | task. |
| 255 | |
| 256 | If you want to do a search-and-replace of a single variable name in different |
| 257 | files, you can do the following in the busybox directory: |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 258 | |
| 259 | $ perl -pi -e 's/\bOldVar\b/new_var/g' *.[ch] |
| 260 | |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 261 | If you want to convert all the non-K&R vars in your file all at once, follow |
| 262 | these steps: |
| 263 | |
Eric Andersen | bdfd0d7 | 2001-10-24 05:00:29 +0000 | [diff] [blame] | 264 | - In the busybox directory type 'examples/mk2knr.pl files-to-convert'. This |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 265 | does not do the actual conversion, rather, it generates a script called |
| 266 | 'convertme.pl' that shows what will be converted, giving you a chance to |
| 267 | review the changes beforehand. |
| 268 | |
| 269 | - Review the 'convertme.pl' script that gets generated in the busybox |
| 270 | directory and remove / edit any of the substitutions in there. Please |
| 271 | especially check for false positives (strings that should not be |
| 272 | converted). |
| 273 | |
| 274 | - Type './convertme.pl same-files-as-before' to perform the actual |
| 275 | conversion. |
| 276 | |
| 277 | - Compile and see if everything still works. |
Eric Andersen | c7bda1c | 2004-03-15 08:29:22 +0000 | [diff] [blame] | 278 | |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 279 | Please be aware of changes that have cascading effects into other files. For |
| 280 | example, if you're changing the name of something in, say utility.c, you |
Eric Andersen | bdfd0d7 | 2001-10-24 05:00:29 +0000 | [diff] [blame] | 281 | should probably run 'examples/mk2knr.pl utility.c' at first, but when you run |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 282 | the 'convertme.pl' script you should run it on _all_ files like so: |
| 283 | './convertme.pl *.[ch]'. |
| 284 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 285 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 286 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 287 | Avoid The Preprocessor |
| 288 | ---------------------- |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 289 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 290 | At best, the preprocessor is a necessary evil, helping us account for platform |
| 291 | and architecture differences. Using the preprocessor unnecessarily is just |
| 292 | plain evil. |
Mark Whitley | 2368a38 | 2000-08-22 00:20:21 +0000 | [diff] [blame] | 293 | |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 294 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 295 | The Folly of #define |
| 296 | ~~~~~~~~~~~~~~~~~~~~ |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 297 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 298 | Use 'const <type> var' for declaring constants. |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 299 | |
| 300 | Don't do this: |
| 301 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 302 | #define var 80 |
| 303 | |
| 304 | Do this instead, when the variable is in a header file and will be used in |
Eric Andersen | c7bda1c | 2004-03-15 08:29:22 +0000 | [diff] [blame] | 305 | several source files: |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 306 | |
Eric Andersen | c7bda1c | 2004-03-15 08:29:22 +0000 | [diff] [blame] | 307 | const int var = 80; |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 308 | |
| 309 | Or do this when the variable is used only in a single source file: |
| 310 | |
Eric Andersen | c7bda1c | 2004-03-15 08:29:22 +0000 | [diff] [blame] | 311 | static const int var = 80; |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 312 | |
| 313 | Declaring variables as '[static] const' gives variables an actual type and |
| 314 | makes the compiler do type checking for you; the preprocessor does _no_ type |
| 315 | checking whatsoever, making it much more error prone. Declaring variables with |
| 316 | '[static] const' also makes debugging programs much easier since the value of |
| 317 | the variable can be easily queried and displayed. |
| 318 | |
| 319 | |
| 320 | The Folly of Macros |
| 321 | ~~~~~~~~~~~~~~~~~~~ |
| 322 | |
| 323 | Use 'static inline' instead of a macro. |
| 324 | |
| 325 | Don't do this: |
| 326 | |
| 327 | #define mini_func(param1, param2) (param1 << param2) |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 328 | |
| 329 | Do this instead: |
| 330 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 331 | static inline int mini_func(int param1, param2) |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 332 | { |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 333 | return (param1 << param2); |
| 334 | } |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 335 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 336 | Static inline functions are greatly preferred over macros. They provide type |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 337 | safety, have no length limitations, no formatting limitations, have an actual |
| 338 | return value, and under gcc they are as cheap as macros. Besides, really long |
| 339 | macros with backslashes at the end of each line are ugly as sin. |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 340 | |
| 341 | |
| 342 | The Folly of #ifdef |
| 343 | ~~~~~~~~~~~~~~~~~~~ |
| 344 | |
| 345 | Code cluttered with ifdefs is difficult to read and maintain. Don't do it. |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 346 | Instead, put your ifdefs at the top of your .c file (or in a header), and |
| 347 | conditionally define 'static inline' functions, (or *maybe* macros), which are |
Eric Andersen | c7bda1c | 2004-03-15 08:29:22 +0000 | [diff] [blame] | 348 | used in the code. |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 349 | |
| 350 | Don't do this: |
| 351 | |
| 352 | ret = my_func(bar, baz); |
| 353 | if (!ret) |
| 354 | return -1; |
Eric Andersen | bdfd0d7 | 2001-10-24 05:00:29 +0000 | [diff] [blame] | 355 | #ifdef CONFIG_FEATURE_FUNKY |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 356 | maybe_do_funky_stuff(bar, baz); |
| 357 | #endif |
| 358 | |
| 359 | Do this instead: |
| 360 | |
| 361 | (in .h header file) |
| 362 | |
Eric Andersen | bdfd0d7 | 2001-10-24 05:00:29 +0000 | [diff] [blame] | 363 | #ifdef CONFIG_FEATURE_FUNKY |
Mark Whitley | d238a7b | 2001-02-09 00:28:59 +0000 | [diff] [blame] | 364 | static inline void maybe_do_funky_stuff (int bar, int baz) |
| 365 | { |
| 366 | /* lotsa code in here */ |
| 367 | } |
| 368 | #else |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 369 | static inline void maybe_do_funky_stuff (int bar, int baz) {} |
| 370 | #endif |
| 371 | |
| 372 | (in the .c source file) |
| 373 | |
| 374 | ret = my_func(bar, baz); |
| 375 | if (!ret) |
| 376 | return -1; |
| 377 | maybe_do_funky_stuff(bar, baz); |
| 378 | |
| 379 | The great thing about this approach is that the compiler will optimize away |
Mark Whitley | d238a7b | 2001-02-09 00:28:59 +0000 | [diff] [blame] | 380 | the "no-op" case (the empty function) when the feature is turned off. |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 381 | |
| 382 | Note also the use of the word 'maybe' in the function name to indicate |
| 383 | conditional execution. |
| 384 | |
| 385 | |
| 386 | |
| 387 | Notes on Strings |
| 388 | ---------------- |
| 389 | |
| 390 | Strings in C can get a little thorny. Here's some guidelines for dealing with |
| 391 | strings in Busybox. (There is surely more that could be added to this |
| 392 | section.) |
| 393 | |
| 394 | |
| 395 | String Files |
| 396 | ~~~~~~~~~~~~ |
| 397 | |
| 398 | Put all help/usage messages in usage.c. Put other strings in messages.c. |
| 399 | Putting these strings into their own file is a calculated decision designed to |
| 400 | confine spelling errors to a single place and aid internationalization |
| 401 | efforts, if needed. (Side Note: we might want to use a single file - maybe |
| 402 | called 'strings.c' - instead of two, food for thought). |
| 403 | |
| 404 | |
| 405 | Testing String Equivalence |
| 406 | ~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 407 | |
| 408 | There's a right way and a wrong way to test for sting equivalence with |
| 409 | strcmp(): |
| 410 | |
| 411 | The wrong way: |
| 412 | |
| 413 | if (!strcmp(string, "foo")) { |
| 414 | ... |
| 415 | |
| 416 | The right way: |
| 417 | |
| 418 | if (strcmp(string, "foo") == 0){ |
| 419 | ... |
| 420 | |
| 421 | The use of the "equals" (==) operator in the latter example makes it much more |
| 422 | obvious that you are testing for equivalence. The former example with the |
| 423 | "not" (!) operator makes it look like you are testing for an error. In a more |
| 424 | perfect world, we would have a streq() function in the string library, but |
| 425 | that ain't the world we're living in. |
| 426 | |
| 427 | |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 428 | Avoid Dangerous String Functions |
| 429 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 430 | |
| 431 | Unfortunately, the way C handles strings makes them prone to overruns when |
| 432 | certain library functions are (mis)used. The following table offers a summary |
| 433 | of some of the more notorious troublemakers: |
| 434 | |
| 435 | function overflows preferred |
| 436 | ---------------------------------------- |
| 437 | strcpy dest string strncpy |
| 438 | strcat dest string strncat |
| 439 | gets string it gets fgets |
| 440 | getwd buf string getcwd |
| 441 | [v]sprintf str buffer [v]snprintf |
| 442 | realpath path buffer use with pathconf |
| 443 | [vf]scanf its arguments just avoid it |
| 444 | |
| 445 | |
| 446 | The above is by no means a complete list. Be careful out there. |
| 447 | |
| 448 | |
| 449 | |
| 450 | Avoid Big Static Buffers |
| 451 | ------------------------ |
| 452 | |
| 453 | First, some background to put this discussion in context: Static buffers look |
| 454 | like this in code: |
| 455 | |
| 456 | /* in a .c file outside any functions */ |
Rob Landley | 6a24976 | 2005-12-20 15:23:20 +0000 | [diff] [blame] | 457 | static char buffer[BUFSIZ]; /* happily used by any function in this file, |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 458 | but ick! big! */ |
| 459 | |
| 460 | The problem with these is that any time any busybox app is run, you pay a |
| 461 | memory penalty for this buffer, even if the applet that uses said buffer is |
| 462 | not run. This can be fixed, thusly: |
| 463 | |
Eric Andersen | d35c215 | 2001-01-25 23:49:09 +0000 | [diff] [blame] | 464 | static char *buffer; |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 465 | ... |
| 466 | other_func() |
| 467 | { |
| 468 | strcpy(buffer, lotsa_chars); /* happily uses global *buffer */ |
| 469 | ... |
| 470 | foo_main() |
| 471 | { |
| 472 | buffer = xmalloc(sizeof(char)*BUFSIZ); |
| 473 | ... |
| 474 | |
| 475 | However, this approach trades bss segment for text segment. Rather than |
| 476 | mallocing the buffers (and thus growing the text size), buffers can be |
| 477 | declared on the stack in the *_main() function and made available globally by |
| 478 | assigning them to a global pointer thusly: |
| 479 | |
Eric Andersen | d35c215 | 2001-01-25 23:49:09 +0000 | [diff] [blame] | 480 | static char *pbuffer; |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 481 | ... |
| 482 | other_func() |
| 483 | { |
| 484 | strcpy(pbuffer, lotsa_chars); /* happily uses global *pbuffer */ |
| 485 | ... |
| 486 | foo_main() |
| 487 | { |
| 488 | char *buffer[BUFSIZ]; /* declared locally, on stack */ |
| 489 | pbuffer = buffer; /* but available globally */ |
| 490 | ... |
| 491 | |
Eric Andersen | d35c215 | 2001-01-25 23:49:09 +0000 | [diff] [blame] | 492 | This last approach has some advantages (low code size, space not used until |
| 493 | it's needed), but can be a problem in some low resource machines that have |
Mark Whitley | d238a7b | 2001-02-09 00:28:59 +0000 | [diff] [blame] | 494 | very limited stack space (e.g., uCLinux). |
| 495 | |
| 496 | A macro is declared in busybox.h that implements compile-time selection |
| 497 | between xmalloc() and stack creation, so you can code the line in question as |
| 498 | |
Eric Andersen | bdfd0d7 | 2001-10-24 05:00:29 +0000 | [diff] [blame] | 499 | RESERVE_CONFIG_BUFFER(buffer, BUFSIZ); |
Mark Whitley | d238a7b | 2001-02-09 00:28:59 +0000 | [diff] [blame] | 500 | |
| 501 | and the right thing will happen, based on your configuration. |
Mark Whitley | a5b55ca | 2001-01-24 00:18:13 +0000 | [diff] [blame] | 502 | |
| 503 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 504 | |
| 505 | Miscellaneous Coding Guidelines |
| 506 | ------------------------------- |
| 507 | |
| 508 | The following are important items that don't fit into any of the above |
| 509 | sections. |
| 510 | |
| 511 | |
| 512 | Model Busybox Applets After GNU Counterparts |
| 513 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 514 | |
| 515 | When in doubt about the proper behavior of a Busybox program (output, |
| 516 | formatting, options, etc.), model it after the equivalent GNU program. |
| 517 | Doesn't matter how that program behaves on some other flavor of *NIX; doesn't |
| 518 | matter what the POSIX standard says or doesn't say, just model Busybox |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 519 | programs after their GNU counterparts and it will make life easier on (nearly) |
| 520 | everyone. |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 521 | |
| 522 | The only time we deviate from emulating the GNU behavior is when: |
| 523 | |
| 524 | - We are deliberately not supporting a feature (such as a command line |
| 525 | switch) |
| 526 | - Emulating the GNU behavior is prohibitively expensive (lots more code |
| 527 | would be required, lots more memory would be used, etc.) |
Eric Andersen | 0730943 | 2000-11-29 22:12:19 +0000 | [diff] [blame] | 528 | - The difference is minor or cosmetic |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 529 | |
| 530 | A note on the 'cosmetic' case: Output differences might be considered |
| 531 | cosmetic, but if the output is significant enough to break other scripts that |
| 532 | use the output, it should really be fixed. |
| 533 | |
| 534 | |
| 535 | Scope |
| 536 | ~~~~~ |
| 537 | |
| 538 | If a const variable is used only in a single source file, put it in the source |
| 539 | file and not in a header file. Likewise, if a const variable is used in only |
| 540 | one function, do not make it global to the file. Instead, declare it inside |
Eric Andersen | 0730943 | 2000-11-29 22:12:19 +0000 | [diff] [blame] | 541 | the function body. Bottom line: Make a conscious effort to limit declarations |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 542 | to the smallest scope possible. |
| 543 | |
| 544 | Inside applet files, all functions should be declared static so as to keep the |
| 545 | global name space clean. The only exception to this rule is the "applet_main" |
| 546 | function which must be declared extern. |
| 547 | |
| 548 | If you write a function that performs a task that could be useful outside the |
| 549 | immediate file, turn it into a general-purpose function with no ties to any |
| 550 | applet and put it in the utility.c file instead. |
| 551 | |
| 552 | |
| 553 | Brackets Are Your Friends |
| 554 | ~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 555 | |
| 556 | Please use brackets on all if and else statements, even if it is only one |
| 557 | line. Example: |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 558 | |
| 559 | Don't do this: |
| 560 | |
| 561 | if (foo) |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 562 | stmt1; |
| 563 | stmt2 |
| 564 | stmt3; |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 565 | |
| 566 | Do this instead: |
| 567 | |
| 568 | if (foo) { |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 569 | stmt1; |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 570 | } |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 571 | stmt2 |
| 572 | stmt3; |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 573 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 574 | The "bracketless" approach is error prone because someday you might add a line |
| 575 | like this: |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 576 | |
| 577 | if (foo) |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 578 | stmt1; |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 579 | new_line(); |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 580 | stmt2 |
| 581 | stmt3; |
Mark Whitley | 40bfc76 | 2000-07-24 22:36:06 +0000 | [diff] [blame] | 582 | |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 583 | And the resulting behavior of your program would totally bewilder you. (Don't |
| 584 | laugh, it happens to us all.) Remember folks, this is C, not Python. |
| 585 | |
| 586 | |
| 587 | Function Declarations |
| 588 | ~~~~~~~~~~~~~~~~~~~~~ |
| 589 | |
| 590 | Do not use old-style function declarations that declare variable types between |
| 591 | the parameter list and opening bracket. Example: |
| 592 | |
| 593 | Don't do this: |
| 594 | |
| 595 | int foo(parm1, parm2) |
| 596 | char parm1; |
| 597 | float parm2; |
| 598 | { |
| 599 | .... |
| 600 | |
| 601 | Do this instead: |
| 602 | |
| 603 | int foo(char parm1, float parm2) |
| 604 | { |
| 605 | .... |
| 606 | |
| 607 | The only time you would ever need to use the old declaration syntax is to |
Eric Andersen | 0730943 | 2000-11-29 22:12:19 +0000 | [diff] [blame] | 608 | support ancient, antediluvian compilers. To our good fortune, we have access |
Mark Whitley | d58ff87 | 2000-11-22 19:25:39 +0000 | [diff] [blame] | 609 | to more modern compilers and the old declaration syntax is neither necessary |
| 610 | nor desired. |
| 611 | |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 612 | |
| 613 | Emphasizing Logical Blocks |
| 614 | ~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 615 | |
| 616 | Organization and readability are improved by putting extra newlines around |
| 617 | blocks of code that perform a single task. These are typically blocks that |
| 618 | begin with a C keyword, but not always. |
| 619 | |
| 620 | Furthermore, you should put a single comment (not necessarily one line, just |
| 621 | one comment) before the block, rather than commenting each and every line. |
Mike Frysinger | 434ccd9 | 2006-07-05 22:19:21 +0000 | [diff] [blame^] | 622 | There is an optimal amount of commenting that a program can have; you can |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 623 | comment too much as well as too little. |
| 624 | |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 625 | A picture is really worth a thousand words here, the following example |
| 626 | illustrates how to emphasize logical blocks: |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 627 | |
| 628 | while (line = get_line_from_file(fp)) { |
| 629 | |
| 630 | /* eat the newline, if any */ |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 631 | chomp(line); |
Mark Whitley | 3680c58 | 2000-12-20 22:35:12 +0000 | [diff] [blame] | 632 | |
| 633 | /* ignore blank lines */ |
| 634 | if (strlen(file_to_act_on) == 0) { |
| 635 | continue; |
| 636 | } |
| 637 | |
| 638 | /* if the search string is in this line, print it, |
| 639 | * unless we were told to be quiet */ |
| 640 | if (strstr(line, search) && !be_quiet) { |
| 641 | puts(line); |
| 642 | } |
| 643 | |
| 644 | /* clean up */ |
| 645 | free(line); |
| 646 | } |
Mark Whitley | 9ead689 | 2001-03-03 00:44:55 +0000 | [diff] [blame] | 647 | |
| 648 | |
| 649 | Processing Options with getopt |
| 650 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 651 | |
| 652 | If your applet needs to process command-line switches, please use getopt() to |
| 653 | do so. Numerous examples can be seen in many of the existing applets, but |
| 654 | basically it boils down to two things: at the top of the .c file, have this |
| 655 | line in the midst of your #includes: |
| 656 | |
| 657 | #include <getopt.h> |
| 658 | |
| 659 | And a code block similar to the following near the top of your applet_main() |
| 660 | routine: |
| 661 | |
Eric Andersen | c7bda1c | 2004-03-15 08:29:22 +0000 | [diff] [blame] | 662 | while ((opt = getopt(argc, argv, "abc")) > 0) { |
Mark Whitley | 9ead689 | 2001-03-03 00:44:55 +0000 | [diff] [blame] | 663 | switch (opt) { |
| 664 | case 'a': |
| 665 | do_a_opt = 1; |
| 666 | break; |
| 667 | case 'b': |
| 668 | do_b_opt = 1; |
| 669 | break; |
| 670 | case 'c': |
| 671 | do_c_opt = 1; |
| 672 | break; |
| 673 | default: |
| 674 | show_usage(); /* in utility.c */ |
| 675 | } |
| 676 | } |
| 677 | |
| 678 | If your applet takes no options (such as 'init'), there should be a line |
| 679 | somewhere in the file reads: |
| 680 | |
| 681 | /* no options, no getopt */ |
| 682 | |
| 683 | That way, when people go grepping to see which applets need to be converted to |
| 684 | use getopt, they won't get false positives. |
| 685 | |
| 686 | Additional Note: Do not use the getopt_long library function and do not try to |
| 687 | hand-roll your own long option parsing. Busybox applets should only support |
Mark Whitley | 7ddaf7c | 2001-03-14 21:04:53 +0000 | [diff] [blame] | 688 | short options. Explanations and examples of the short options should be |
| 689 | documented in usage.h. |