diff options
| author | Denis Vlasenko <vda.linux@googlemail.com> | 2007-04-15 08:39:39 +0000 |
|---|---|---|
| committer | Denis Vlasenko <vda.linux@googlemail.com> | 2007-04-15 08:39:39 +0000 |
| commit | 91de7c0328d1ab3f32c8b9eb4bc6c3cd9cdf0b23 (patch) | |
| tree | 36dc35ce7c130b26220b191519be6e456beada9b /docs/style-guide.txt | |
| parent | 58394b1e299f7207088f077c02357749c3a3d853 (diff) | |
| download | busybox-w32-91de7c0328d1ab3f32c8b9eb4bc6c3cd9cdf0b23.tar.gz busybox-w32-91de7c0328d1ab3f32c8b9eb4bc6c3cd9cdf0b23.tar.bz2 busybox-w32-91de7c0328d1ab3f32c8b9eb4bc6c3cd9cdf0b23.zip | |
update style-guide.txt
Diffstat (limited to 'docs/style-guide.txt')
| -rw-r--r-- | docs/style-guide.txt | 133 |
1 files changed, 79 insertions, 54 deletions
diff --git a/docs/style-guide.txt b/docs/style-guide.txt index ba0cdbaa4..5bb3441cd 100644 --- a/docs/style-guide.txt +++ b/docs/style-guide.txt | |||
| @@ -20,7 +20,7 @@ in the directory, just your own. | |||
| 20 | Declaration Order | 20 | Declaration Order |
| 21 | ----------------- | 21 | ----------------- |
| 22 | 22 | ||
| 23 | Here is the order in which code should be laid out in a file: | 23 | Here is the preferred order in which code should be laid out in a file: |
| 24 | 24 | ||
| 25 | - commented program name and one-line description | 25 | - commented program name and one-line description |
| 26 | - commented author name and email address(es) | 26 | - commented author name and email address(es) |
| @@ -126,14 +126,15 @@ between it and the opening control block statement. Examples: | |||
| 126 | 126 | ||
| 127 | do { | 127 | do { |
| 128 | 128 | ||
| 129 | Exceptions: | 129 | If you have long logic statements that need to be wrapped, then uncuddling |
| 130 | 130 | the bracket to improve readability is allowed. Generally, this style makes | |
| 131 | - if you have long logic statements that need to be wrapped, then uncuddling | 131 | it easier for reader to notice that 2nd and following lines are still |
| 132 | the bracket to improve readability is allowed: | 132 | inside 'if': |
| 133 | 133 | ||
| 134 | if (some_really_long_checks && some_other_really_long_checks \ | 134 | if (some_really_long_checks && some_other_really_long_checks |
| 135 | && some_more_really_long_checks) | 135 | && some_more_really_long_checks |
| 136 | { | 136 | && even_more_of_long_checks |
| 137 | ) { | ||
| 137 | do_foo_now; | 138 | do_foo_now; |
| 138 | 139 | ||
| 139 | Spacing around Parentheses | 140 | Spacing around Parentheses |
| @@ -208,6 +209,23 @@ block. Example: | |||
| 208 | } | 209 | } |
| 209 | 210 | ||
| 210 | 211 | ||
| 212 | Labels | ||
| 213 | ~~~~~~ | ||
| 214 | |||
| 215 | Labels should start at the beginning of the line, not indented to the block | ||
| 216 | level (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 | ||
| 226 | name, but it's not a policy of busybox project to enforce such a minor detail). | ||
| 227 | |||
| 228 | |||
| 211 | 229 | ||
| 212 | Variable and Function Names | 230 | Variable and Function Names |
| 213 | --------------------------- | 231 | --------------------------- |
| @@ -234,7 +252,7 @@ because it looks like whitespace; using lower-case is easy on the eyes. | |||
| 234 | Exceptions: | 252 | Exceptions: |
| 235 | 253 | ||
| 236 | - Enums, macros, and constant variables are occasionally written in all | 254 | - Enums, macros, and constant variables are occasionally written in all |
| 237 | upper-case with words optionally seperatedy by underscores (i.e. FIFOTYPE, | 255 | upper-case with words optionally seperatedy by underscores (i.e. FIFO_TYPE, |
| 238 | ISBLKDEV()). | 256 | ISBLKDEV()). |
| 239 | 257 | ||
| 240 | - Nobody is going to get mad at you for using 'pvar' as the name of a | 258 | - Nobody is going to get mad at you for using 'pvar' as the name of a |
| @@ -299,22 +317,21 @@ Use 'const <type> var' for declaring constants. | |||
| 299 | 317 | ||
| 300 | Don't do this: | 318 | Don't do this: |
| 301 | 319 | ||
| 302 | #define var 80 | 320 | #define CONST 80 |
| 303 | 321 | ||
| 304 | Do this instead, when the variable is in a header file and will be used in | 322 | Do this instead, when the variable is in a header file and will be used in |
| 305 | several source files: | 323 | several source files: |
| 306 | 324 | ||
| 307 | const int var = 80; | 325 | enum { CONST = 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 | 326 | ||
| 313 | Declaring variables as '[static] const' gives variables an actual type and | 327 | Although enum may look ugly to some people, it is better for code size. |
| 314 | makes the compiler do type checking for you; the preprocessor does _no_ type | 328 | With "const int" compiler may fail to optimize it out and will reserve |
| 315 | checking whatsoever, making it much more error prone. Declaring variables with | 329 | a real storage in rodata for it! (Hopefully, newer gcc will get better |
| 316 | '[static] const' also makes debugging programs much easier since the value of | 330 | at it...). With "define", you have slight risk of polluting namespace |
| 317 | the variable can be easily queried and displayed. | 331 | (#define doesn't allow you to redefine the name in the inner scopes), |
| 332 | and complex "define" are evaluated each time they uesd, not once | ||
| 333 | at declarations like enums. Also, the preprocessor does _no_ type checking | ||
| 334 | whatsoever, making it much more error prone. | ||
| 318 | 335 | ||
| 319 | 336 | ||
| 320 | The Folly of Macros | 337 | The Folly of Macros |
| @@ -432,15 +449,16 @@ 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 | 449 | certain library functions are (mis)used. The following table offers a summary |
| 433 | of some of the more notorious troublemakers: | 450 | of some of the more notorious troublemakers: |
| 434 | 451 | ||
| 435 | function overflows preferred | 452 | function overflows preferred |
| 436 | ---------------------------------------- | 453 | ------------------------------------------------- |
| 437 | strcpy dest string strncpy | 454 | strcpy dest string safe_strncpy |
| 438 | strcat dest string strncat | 455 | strncpy may fail to 0-terminate dst safe_strncpy |
| 439 | gets string it gets fgets | 456 | strcat dest string strncat |
| 440 | getwd buf string getcwd | 457 | gets string it gets fgets |
| 441 | [v]sprintf str buffer [v]snprintf | 458 | getwd buf string getcwd |
| 442 | realpath path buffer use with pathconf | 459 | [v]sprintf str buffer [v]snprintf |
| 443 | [vf]scanf its arguments just avoid it | 460 | realpath path buffer use with pathconf |
| 461 | [vf]scanf its arguments just avoid it | ||
| 444 | 462 | ||
| 445 | 463 | ||
| 446 | The above is by no means a complete list. Be careful out there. | 464 | The above is by no means a complete list. Be careful out there. |
| @@ -450,7 +468,7 @@ The above is by no means a complete list. Be careful out there. | |||
| 450 | Avoid Big Static Buffers | 468 | Avoid Big Static Buffers |
| 451 | ------------------------ | 469 | ------------------------ |
| 452 | 470 | ||
| 453 | First, some background to put this discussion in context: Static buffers look | 471 | First, some background to put this discussion in context: static buffers look |
| 454 | like this in code: | 472 | like this in code: |
| 455 | 473 | ||
| 456 | /* in a .c file outside any functions */ | 474 | /* in a .c file outside any functions */ |
| @@ -500,6 +518,9 @@ between xmalloc() and stack creation, so you can code the line in question as | |||
| 500 | 518 | ||
| 501 | and the right thing will happen, based on your configuration. | 519 | and the right thing will happen, based on your configuration. |
| 502 | 520 | ||
| 521 | Another relatively new trick of similar nature is explained | ||
| 522 | in keep_data_small.txt. | ||
| 523 | |||
| 503 | 524 | ||
| 504 | 525 | ||
| 505 | Miscellaneous Coding Guidelines | 526 | Miscellaneous Coding Guidelines |
| @@ -527,7 +548,7 @@ The only time we deviate from emulating the GNU behavior is when: | |||
| 527 | would be required, lots more memory would be used, etc.) | 548 | would be required, lots more memory would be used, etc.) |
| 528 | - The difference is minor or cosmetic | 549 | - The difference is minor or cosmetic |
| 529 | 550 | ||
| 530 | A note on the 'cosmetic' case: Output differences might be considered | 551 | 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 | 552 | cosmetic, but if the output is significant enough to break other scripts that |
| 532 | use the output, it should really be fixed. | 553 | use the output, it should really be fixed. |
| 533 | 554 | ||
| @@ -577,7 +598,7 @@ like this: | |||
| 577 | if (foo) | 598 | if (foo) |
| 578 | stmt1; | 599 | stmt1; |
| 579 | new_line(); | 600 | new_line(); |
| 580 | stmt2 | 601 | stmt2; |
| 581 | stmt3; | 602 | stmt3; |
| 582 | 603 | ||
| 583 | And the resulting behavior of your program would totally bewilder you. (Don't | 604 | And the resulting behavior of your program would totally bewilder you. (Don't |
| @@ -625,7 +646,7 @@ comment too much as well as too little. | |||
| 625 | A picture is really worth a thousand words here, the following example | 646 | A picture is really worth a thousand words here, the following example |
| 626 | illustrates how to emphasize logical blocks: | 647 | illustrates how to emphasize logical blocks: |
| 627 | 648 | ||
| 628 | while (line = get_line_from_file(fp)) { | 649 | while (line = xmalloc_fgets(fp)) { |
| 629 | 650 | ||
| 630 | /* eat the newline, if any */ | 651 | /* eat the newline, if any */ |
| 631 | chomp(line); | 652 | chomp(line); |
| @@ -649,31 +670,38 @@ illustrates how to emphasize logical blocks: | |||
| 649 | Processing Options with getopt | 670 | Processing Options with getopt |
| 650 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | 671 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |
| 651 | 672 | ||
| 652 | If your applet needs to process command-line switches, please use getopt() to | 673 | If your applet needs to process command-line switches, please use getopt32() to |
| 653 | do so. Numerous examples can be seen in many of the existing applets, but | 674 | 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 | 675 | basically it boils down to two things: at the top of the .c file, have this |
| 655 | line in the midst of your #includes: | 676 | line in the midst of your #includes, if you need to parse long options: |
| 656 | 677 | ||
| 657 | #include <getopt.h> | 678 | #include <getopt.h> |
| 658 | 679 | ||
| 680 | Then 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 } | ||
| 686 | }; | ||
| 687 | |||
| 659 | And a code block similar to the following near the top of your applet_main() | 688 | And a code block similar to the following near the top of your applet_main() |
| 660 | routine: | 689 | routine: |
| 661 | 690 | ||
| 662 | while ((opt = getopt(argc, argv, "abc")) > 0) { | 691 | char *str_b; |
| 663 | switch (opt) { | 692 | |
| 664 | case 'a': | 693 | opt_complementary = "cryptic_string"; |
| 665 | do_a_opt = 1; | 694 | applet_long_options = <applet>_long_options; /* if you have them */ |
| 666 | break; | 695 | opt = getopt32(argc, argv, "ab:c", &str_b); |
| 667 | case 'b': | 696 | if (opt & 1) { |
| 668 | do_b_opt = 1; | 697 | handle_option_a(); |
| 669 | break; | 698 | } |
| 670 | case 'c': | 699 | if (opt & 2) { |
| 671 | do_c_opt = 1; | 700 | handle_option_b(str_b); |
| 672 | break; | 701 | } |
| 673 | default: | 702 | if (opt & 4) { |
| 674 | show_usage(); /* in utility.c */ | 703 | handle_option_c(); |
| 675 | } | 704 | } |
| 676 | } | ||
| 677 | 705 | ||
| 678 | If your applet takes no options (such as 'init'), there should be a line | 706 | If your applet takes no options (such as 'init'), there should be a line |
| 679 | somewhere in the file reads: | 707 | somewhere in the file reads: |
| @@ -683,7 +711,4 @@ somewhere in the file reads: | |||
| 683 | That way, when people go grepping to see which applets need to be converted to | 711 | That way, when people go grepping to see which applets need to be converted to |
| 684 | use getopt, they won't get false positives. | 712 | use getopt, they won't get false positives. |
| 685 | 713 | ||
| 686 | Additional Note: Do not use the getopt_long library function and do not try to | 714 | For more info and examples, examine getopt32.c, tar.c, wget.c etc. |
| 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. | ||