Contents of /trunk/mkinitrd-magellan/busybox/docs/style-guide.txt
Parent Directory
|
Revision Log
Revision 532 -
(show annotations)
(download)
Sat Sep 1 22:45:15 2007 UTC (16 years, 9 months ago) by niro
File MIME type: text/plain
File size: 19163 byte(s)
Sat Sep 1 22:45:15 2007 UTC (16 years, 9 months ago) by niro
File MIME type: text/plain
File size: 19163 byte(s)
-import if magellan mkinitrd; it is a fork of redhats mkinitrd-5.0.8 with all magellan patches and features; deprecates magellan-src/mkinitrd
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 | |
18 | |
19 | |
20 | Declaration Order |
21 | ----------------- |
22 | |
23 | Here is the order in which code should be laid out in a file: |
24 | |
25 | - commented program name and one-line description |
26 | - commented author name and email address(es) |
27 | - commented GPL boilerplate |
28 | - commented longer description / notes for the program (if needed) |
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") |
32 | - const and global variables |
33 | - function declarations (if necessary) |
34 | - function implementations |
35 | |
36 | |
37 | |
38 | Whitespace and Formatting |
39 | ------------------------- |
40 | |
41 | This is everybody's favorite flame topic so let's get it out of the way right |
42 | up front. |
43 | |
44 | |
45 | Tabs vs. Spaces in Line Indentation |
46 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
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.: |
53 | |
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 */ |
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 |
64 | lines is that you can set your editor to display tabs at *whatever* number of |
65 | spaces is desired and the code will still look fine. |
66 | |
67 | |
68 | Operator Spacing |
69 | ~~~~~~~~~~~~~~~~ |
70 | |
71 | Put spaces between terms and operators. Example: |
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 |
84 | single term (even if it is a compound term) such as: |
85 | |
86 | if (str[idx] == '/' && str[idx-1] != '\\') |
87 | |
88 | or |
89 | |
90 | if ((argc-1) - (optind+1) > 0) |
91 | |
92 | |
93 | Bracket Spacing |
94 | ~~~~~~~~~~~~~~~ |
95 | |
96 | If an opening bracket starts a function, it should be on the |
97 | next line with no spacing before it. However, if a bracket follows an opening |
98 | control block, it should be on the same line with a single space (not a tab) |
99 | between it and the opening control block statement. Examples: |
100 | |
101 | Don't do this: |
102 | |
103 | while (!done) |
104 | { |
105 | |
106 | do |
107 | { |
108 | |
109 | Don't do this either: |
110 | |
111 | while (!done){ |
112 | |
113 | do{ |
114 | |
115 | And for heaven's sake, don't do this: |
116 | |
117 | while (!done) |
118 | { |
119 | |
120 | do |
121 | { |
122 | |
123 | Do this instead: |
124 | |
125 | while (!done) { |
126 | |
127 | do { |
128 | |
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; |
138 | |
139 | Spacing around Parentheses |
140 | ~~~~~~~~~~~~~~~~~~~~~~~~~~ |
141 | |
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: |
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 | |
156 | But do functions like this: |
157 | |
158 | static int my_func(int foo, char bar) |
159 | ... |
160 | baz = my_func(1, 2); |
161 | |
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 | |
175 | |
176 | Cuddled Elses |
177 | ~~~~~~~~~~~~~ |
178 | |
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. |
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 | |
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 | |
210 | |
211 | |
212 | Variable and Function Names |
213 | --------------------------- |
214 | |
215 | Use the K&R style with names in all lower-case and underscores occasionally |
216 | used to separate words (e.g., "variable_name" and "numchars" are both |
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 | |
220 | Frowned upon: |
221 | |
222 | hitList |
223 | TotalChars |
224 | szFileName |
225 | pf_Nfol_TriState |
226 | |
227 | Preferred: |
228 | |
229 | hit_list |
230 | total_chars |
231 | file_name |
232 | sensible_name |
233 | |
234 | Exceptions: |
235 | |
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()). |
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'. |
242 | |
243 | |
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: |
258 | |
259 | $ perl -pi -e 's/\bOldVar\b/new_var/g' *.[ch] |
260 | |
261 | If you want to convert all the non-K&R vars in your file all at once, follow |
262 | these steps: |
263 | |
264 | - In the busybox directory type 'examples/mk2knr.pl files-to-convert'. This |
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. |
278 | |
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 |
281 | should probably run 'examples/mk2knr.pl utility.c' at first, but when you run |
282 | the 'convertme.pl' script you should run it on _all_ files like so: |
283 | './convertme.pl *.[ch]'. |
284 | |
285 | |
286 | |
287 | Avoid The Preprocessor |
288 | ---------------------- |
289 | |
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. |
293 | |
294 | |
295 | The Folly of #define |
296 | ~~~~~~~~~~~~~~~~~~~~ |
297 | |
298 | Use 'const <type> var' for declaring constants. |
299 | |
300 | Don't do this: |
301 | |
302 | #define var 80 |
303 | |
304 | Do this instead, when the variable is in a header file and will be used in |
305 | several source files: |
306 | |
307 | const int var = 80; |
308 | |
309 | Or do this when the variable is used only in a single source file: |
310 | |
311 | static const int var = 80; |
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) |
328 | |
329 | Do this instead: |
330 | |
331 | static inline int mini_func(int param1, param2) |
332 | { |
333 | return (param1 << param2); |
334 | } |
335 | |
336 | Static inline functions are greatly preferred over macros. They provide type |
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. |
340 | |
341 | |
342 | The Folly of #ifdef |
343 | ~~~~~~~~~~~~~~~~~~~ |
344 | |
345 | Code cluttered with ifdefs is difficult to read and maintain. Don't do it. |
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 |
348 | used in the code. |
349 | |
350 | Don't do this: |
351 | |
352 | ret = my_func(bar, baz); |
353 | if (!ret) |
354 | return -1; |
355 | #ifdef CONFIG_FEATURE_FUNKY |
356 | maybe_do_funky_stuff(bar, baz); |
357 | #endif |
358 | |
359 | Do this instead: |
360 | |
361 | (in .h header file) |
362 | |
363 | #ifdef CONFIG_FEATURE_FUNKY |
364 | static inline void maybe_do_funky_stuff (int bar, int baz) |
365 | { |
366 | /* lotsa code in here */ |
367 | } |
368 | #else |
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 |
380 | the "no-op" case (the empty function) when the feature is turned off. |
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 | |
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 */ |
457 | static char buffer[BUFSIZ]; /* happily used by any function in this file, |
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 | |
464 | static char *buffer; |
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 | |
480 | static char *pbuffer; |
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 | |
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 |
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 | |
499 | RESERVE_CONFIG_BUFFER(buffer, BUFSIZ); |
500 | |
501 | and the right thing will happen, based on your configuration. |
502 | |
503 | |
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 |
519 | programs after their GNU counterparts and it will make life easier on (nearly) |
520 | everyone. |
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.) |
528 | - The difference is minor or cosmetic |
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 |
541 | the function body. Bottom line: Make a conscious effort to limit declarations |
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: |
558 | |
559 | Don't do this: |
560 | |
561 | if (foo) |
562 | stmt1; |
563 | stmt2 |
564 | stmt3; |
565 | |
566 | Do this instead: |
567 | |
568 | if (foo) { |
569 | stmt1; |
570 | } |
571 | stmt2 |
572 | stmt3; |
573 | |
574 | The "bracketless" approach is error prone because someday you might add a line |
575 | like this: |
576 | |
577 | if (foo) |
578 | stmt1; |
579 | new_line(); |
580 | stmt2 |
581 | stmt3; |
582 | |
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 |
608 | support ancient, antediluvian compilers. To our good fortune, we have access |
609 | to more modern compilers and the old declaration syntax is neither necessary |
610 | nor desired. |
611 | |
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. |
622 | There is an optimal amount of commenting that a program can have; you can |
623 | comment too much as well as too little. |
624 | |
625 | A picture is really worth a thousand words here, the following example |
626 | illustrates how to emphasize logical blocks: |
627 | |
628 | while (line = get_line_from_file(fp)) { |
629 | |
630 | /* eat the newline, if any */ |
631 | chomp(line); |
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 | } |
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 | |
662 | while ((opt = getopt(argc, argv, "abc")) > 0) { |
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 |
688 | short options. Explanations and examples of the short options should be |
689 | documented in usage.h. |