X-Git-Url: https://git.librecmc.org/?a=blobdiff_plain;f=docs%2Fstyle-guide.txt;h=7560d698623357a12b3ead68a05eb00ecfbab8bd;hb=33f85eeac5a7babc996cacce4485326d46b6e54d;hp=ddf3ebb83ba27acdcdd6c729ee5f9549496c13af;hpb=e78fd0f7211121cc49b60cede1c2a5273cfcfe29;p=oweals%2Fbusybox.git diff --git a/docs/style-guide.txt b/docs/style-guide.txt index ddf3ebb83..7560d6986 100644 --- a/docs/style-guide.txt +++ b/docs/style-guide.txt @@ -20,13 +20,15 @@ in the directory, just your own. Declaration Order ----------------- -Here is the order in which code should be laid out in a file: +Here is the preferred order in which code should be laid out in a file: - commented program name and one-line description - commented author name and email address(es) - commented GPL boilerplate - commented longer description / notes for the program (if needed) - - #includes and #defines + - #includes of .h files with angle brackets (<>) around them + - #includes of .h files with quotes ("") around them + - #defines (if any, note the section below titled "Avoid the Preprocessor") - const and global variables - function declarations (if necessary) - function implementations @@ -49,13 +51,13 @@ indentation style in the Apache and Postfix source does this sort of thing: \s\s\s\sif (expr) {\n\tstmt; --ick.) The only exception to this rule is multi-line comments that use an asterisk at the beginning of each line, i.e.: - /t/* - /t * This is a block comment. - /t * Note that it has multiple lines - /t * and that the beginning of each line has a tab plus a space - /t * except for the opening '/*' line where the slash - /t * is used instead of a space. - /t */ + \t/* + \t * This is a block comment. + \t * Note that it has multiple lines + \t * and that the beginning of each line has a tab plus a space + \t * except for the opening '/*' line where the slash + \t * is used instead of a space. + \t */ Furthermore, The preference is that tabs be set to display at four spaces wide, but the beauty of using only tabs (and not spaces) at the beginning of @@ -124,13 +126,23 @@ between it and the opening control block statement. Examples: do { +If you have long logic statements that need to be wrapped, then uncuddling +the bracket to improve readability is allowed. Generally, this style makes +it easier for reader to notice that 2nd and following lines are still +inside 'if': + + if (some_really_long_checks && some_other_really_long_checks + && some_more_really_long_checks + && even_more_of_long_checks + ) { + do_foo_now; Spacing around Parentheses ~~~~~~~~~~~~~~~~~~~~~~~~~~ -Put a space between C keywords and left parens, but not between -function names and the left paren that starts it's parameter list (whether it -is being declared or called). Examples: +Put a space between C keywords and left parens, but not between function names +and the left paren that starts it's parameter list (whether it is being +declared or called). Examples: Don't do this: @@ -197,6 +209,22 @@ block. Example: } +Labels +~~~~~~ + +Labels should start at the beginning of the line, not indented to the block +level (because they do not "belong" to block scope, only to whole function). + + if (foo) { + stmt; + label: + stmt2; + stmt; + } + +(Putting label at position 1 prevents diff -p from confusing label for function +name, but it's not a policy of busybox project to enforce such a minor detail). + Variable and Function Names @@ -223,28 +251,55 @@ because it looks like whitespace; using lower-case is easy on the eyes. Exceptions: - - Enums, macros, and constant variables should all be in upper-case with - words optionally seperatedy by underscores (i.e. FIFOTYPE, ISBLKDEV()). + - Enums, macros, and constant variables are occasionally written in all + upper-case with words optionally seperatedy by underscores (i.e. FIFO_TYPE, + ISBLKDEV()). - Nobody is going to get mad at you for using 'pvar' as the name of a variable that is a pointer to 'var'. -Note: The Busybox codebase is very much a mixture of code gathered from a -variety of sources. This explains why the current codebase contains such a -hodge-podge of different naming styles (Java, Pascal, K&R, just-plain-weird, -etc.). The K&R guideline explained above should therefore be used on new files -that are added to the repository. Furthermore, the maintainer of an existing -file that uses alternate naming conventions should -- at his own convenience --- convert those names over to K&R style; converting variable names is a very -low priority task. Perhaps in the future we will include some magical Perl -script that can go through and convert variable names, left as an exercise for -the reader for now. -For the time being, if you want to do a search-and-replace of a variable name -in different files, do the following in the busybox directory: +Converting to K&R +~~~~~~~~~~~~~~~~~ + +The Busybox codebase is very much a mixture of code gathered from a variety of +sources. This explains why the current codebase contains such a hodge-podge of +different naming styles (Java, Pascal, K&R, just-plain-weird, etc.). The K&R +guideline explained above should therefore be used on new files that are added +to the repository. Furthermore, the maintainer of an existing file that uses +alternate naming conventions should, at his own convenience, convert those +names over to K&R style. Converting variable names is a very low priority +task. + +If you want to do a search-and-replace of a single variable name in different +files, you can do the following in the busybox directory: $ perl -pi -e 's/\bOldVar\b/new_var/g' *.[ch] +If you want to convert all the non-K&R vars in your file all at once, follow +these steps: + + - In the busybox directory type 'examples/mk2knr.pl files-to-convert'. This + does not do the actual conversion, rather, it generates a script called + 'convertme.pl' that shows what will be converted, giving you a chance to + review the changes beforehand. + + - Review the 'convertme.pl' script that gets generated in the busybox + directory and remove / edit any of the substitutions in there. Please + especially check for false positives (strings that should not be + converted). + + - Type './convertme.pl same-files-as-before' to perform the actual + conversion. + + - Compile and see if everything still works. + +Please be aware of changes that have cascading effects into other files. For +example, if you're changing the name of something in, say utility.c, you +should probably run 'examples/mk2knr.pl utility.c' at first, but when you run +the 'convertme.pl' script you should run it on _all_ files like so: +'./convertme.pl *.[ch]'. + Avoid The Preprocessor @@ -262,22 +317,21 @@ Use 'const var' for declaring constants. Don't do this: - #define var 80 + #define CONST 80 Do this instead, when the variable is in a header file and will be used in - several source files: - - const int var = 80; + several source files: - Or do this when the variable is used only in a single source file: + enum { CONST = 80 }; - static const int var = 80; - -Declaring variables as '[static] const' gives variables an actual type and -makes the compiler do type checking for you; the preprocessor does _no_ type -checking whatsoever, making it much more error prone. Declaring variables with -'[static] const' also makes debugging programs much easier since the value of -the variable can be easily queried and displayed. +Although enum may look ugly to some people, it is better for code size. +With "const int" compiler may fail to optimize it out and will reserve +a real storage in rodata for it! (Hopefully, newer gcc will get better +at it...). With "define", you have slight risk of polluting namespace +(#define doesn't allow you to redefine the name in the inner scopes), +and complex "define" are evaluated each time they uesd, not once +at declarations like enums. Also, the preprocessor does _no_ type checking +whatsoever, making it much more error prone. The Folly of Macros @@ -297,24 +351,25 @@ Use 'static inline' instead of a macro. } Static inline functions are greatly preferred over macros. They provide type -safety, have no length limitations, no formatting limitations, and under gcc -they are as cheap as macros. Besides, really long macros with backslashes at -the end of each line are ugly as sin. +safety, have no length limitations, no formatting limitations, have an actual +return value, and under gcc they are as cheap as macros. Besides, really long +macros with backslashes at the end of each line are ugly as sin. The Folly of #ifdef ~~~~~~~~~~~~~~~~~~~ Code cluttered with ifdefs is difficult to read and maintain. Don't do it. -Instead, put your ifdefs in a header, and conditionally define 'static inline' -functions, (or *maybe* macros), which are used in the code. +Instead, put your ifdefs at the top of your .c file (or in a header), and +conditionally define 'static inline' functions, (or *maybe* macros), which are +used in the code. Don't do this: ret = my_func(bar, baz); if (!ret) return -1; - #ifdef BB_FEATURE_FUNKY + #ifdef CONFIG_FEATURE_FUNKY maybe_do_funky_stuff(bar, baz); #endif @@ -322,13 +377,13 @@ functions, (or *maybe* macros), which are used in the code. (in .h header file) - #ifdef BB_FEATURE_FUNKY - static inline void maybe_do_funky_stuff (int bar, int baz) + #if ENABLE_FEATURE_FUNKY + static inline void maybe_do_funky_stuff(int bar, int baz) { /* lotsa code in here */ } #else - static inline void maybe_do_funky_stuff (int bar, int baz) {} + static inline void maybe_do_funky_stuff(int bar, int baz) {} #endif (in the .c source file) @@ -394,15 +449,16 @@ Unfortunately, the way C handles strings makes them prone to overruns when certain library functions are (mis)used. The following table offers a summary of some of the more notorious troublemakers: -function overflows preferred ----------------------------------------- -strcpy dest string strncpy -strcat dest string strncat -gets string it gets fgets -getwd buf string getcwd -[v]sprintf str buffer [v]snprintf -realpath path buffer use with pathconf -[vf]scanf its arguments just avoid it +function overflows preferred +------------------------------------------------- +strcpy dest string safe_strncpy +strncpy may fail to 0-terminate dst safe_strncpy +strcat dest string strncat +gets string it gets fgets +getwd buf string getcwd +[v]sprintf str buffer [v]snprintf +realpath path buffer use with pathconf +[vf]scanf its arguments just avoid it The above is by no means a complete list. Be careful out there. @@ -412,11 +468,11 @@ The above is by no means a complete list. Be careful out there. Avoid Big Static Buffers ------------------------ -First, some background to put this discussion in context: Static buffers look +First, some background to put this discussion in context: static buffers look like this in code: /* in a .c file outside any functions */ - static char *buffer[BUFSIZ]; /* happily used by any function in this file, + static char buffer[BUFSIZ]; /* happily used by any function in this file, but ick! big! */ The problem with these is that any time any busybox app is run, you pay a @@ -458,10 +514,13 @@ very limited stack space (e.g., uCLinux). A macro is declared in busybox.h that implements compile-time selection between xmalloc() and stack creation, so you can code the line in question as - RESERVE_BB_BUFFER(buffer, BUFSIZ); + RESERVE_CONFIG_BUFFER(buffer, BUFSIZ); and the right thing will happen, based on your configuration. +Another relatively new trick of similar nature is explained +in keep_data_small.txt. + Miscellaneous Coding Guidelines @@ -478,7 +537,8 @@ When in doubt about the proper behavior of a Busybox program (output, formatting, options, etc.), model it after the equivalent GNU program. Doesn't matter how that program behaves on some other flavor of *NIX; doesn't matter what the POSIX standard says or doesn't say, just model Busybox -programs after their GNU counterparts and nobody has to get hurt. +programs after their GNU counterparts and it will make life easier on (nearly) +everyone. The only time we deviate from emulating the GNU behavior is when: @@ -488,7 +548,7 @@ The only time we deviate from emulating the GNU behavior is when: would be required, lots more memory would be used, etc.) - The difference is minor or cosmetic -A note on the 'cosmetic' case: Output differences might be considered +A note on the 'cosmetic' case: output differences might be considered cosmetic, but if the output is significant enough to break other scripts that use the output, it should really be fixed. @@ -538,7 +598,7 @@ like this: if (foo) stmt1; new_line(); - stmt2 + stmt2; stmt3; And the resulting behavior of your program would totally bewilder you. (Don't @@ -580,18 +640,16 @@ begin with a C keyword, but not always. Furthermore, you should put a single comment (not necessarily one line, just one comment) before the block, rather than commenting each and every line. -There is an optimal ammount of commenting that a program can have; you can +There is an optimal amount of commenting that a program can have; you can comment too much as well as too little. -A picture is really worth a thousand words here, so here is an example that -illustrates emphasizing logical blocks: +A picture is really worth a thousand words here, the following example +illustrates how to emphasize logical blocks: - while (line = get_line_from_file(fp)) { + while (line = xmalloc_fgets(fp)) { /* eat the newline, if any */ - if (line[strlen(line)-1] == '\n') { - line[strlen(line)-1] = '\0'; - } + chomp(line); /* ignore blank lines */ if (strlen(file_to_act_on) == 0) { @@ -609,21 +667,48 @@ illustrates emphasizing logical blocks: } -Testing Guidelines -~~~~~~~~~~~~~~~~~~ +Processing Options with getopt +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If your applet needs to process command-line switches, please use getopt32() to +do so. Numerous examples can be seen in many of the existing applets, but +basically it boils down to two things: at the top of the .c file, have this +line in the midst of your #includes, if you need to parse long options: + + #include -It's considered good form to test your new feature before you submit a patch -to the mailing list, and especially before you commit a change to CVS. Here -are some guidelines on testing your changes. +Then have long options defined: + + static const struct option _long_options[] = { + { "list", 0, NULL, 't' }, + { "extract", 0, NULL, 'x' }, + { NULL, 0, NULL, 0 } + }; + +And a code block similar to the following near the top of your applet_main() +routine: + + char *str_b; + + opt_complementary = "cryptic_string"; + applet_long_options = _long_options; /* if you have them */ + opt = getopt32(argc, argv, "ab:c", &str_b); + if (opt & 1) { + handle_option_a(); + } + if (opt & 2) { + handle_option_b(str_b); + } + if (opt & 4) { + handle_option_c(); + } - - Always test busybox applets against GNU counterparts and make sure the - behavior / output is identical between the two. +If your applet takes no options (such as 'init'), there should be a line +somewhere in the file reads: - - Try several different permutations and combinations of the features you're - adding and make sure they all work. (Make sure one feature does not - interfere with another, etc.) + /* no options, no getopt */ - - Make sure you test compiling against the source both with the feature - turned on and turned off in Config.h and make sure busybox compiles cleanly - both ways. +That way, when people go grepping to see which applets need to be converted to +use getopt, they won't get false positives. +For more info and examples, examine getopt32.c, tar.c, wget.c etc.