source: branches/3.2/mindi-busybox/docs/style-guide.txt @ 3186

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