X-Git-Url: http://www.privoxy.org/gitweb/?p=privoxy.git;a=blobdiff_plain;f=doc%2Fwebserver%2Fdeveloper-manual%2Fcoding.html;h=eff19bdbe1aa38beca94ae10adbc0fafccde69a2;hp=656a97dcc3013ea0c2f39714874bedfe0eba2826;hb=7d0d8bdd53947864c64d968062ca132b65f2e162;hpb=bae19e58effcafe0193c7ed8862373e1209690fd diff --git a/doc/webserver/developer-manual/coding.html b/doc/webserver/developer-manual/coding.html index 656a97dc..eff19bdb 100644 --- a/doc/webserver/developer-manual/coding.html +++ b/doc/webserver/developer-manual/coding.html @@ -1,301 +1,193 @@ - -
This set of standards is designed to make our lives easier. It is - developed with the simple goal of helping us keep the "new and improved - Privoxy" consistent and reliable. Thus making - maintenance easier and increasing chances of success of the - project.
And that of course comes back to us as individuals. If we can - increase our development and product efficiencies then we can solve more - of the request for changes/improvements and in general feel good about - ourselves. ;->
Explanation:
Comment as much as possible without commenting the obvious. - For example do not comment "variable_a is equal to variable_b". - Instead explain why variable_a should be equal to the variable_b. - Just because a person can read code does not mean they will - understand why or what is being done. A reader may spend a lot - more time figuring out what is going on when a simple comment - or explanation would have prevented the extra research. Please - help your brother IJB'ers out!
The comments will also help justify the intent of the code. - If the comment describes something different than what the code - is doing then maybe a programming error is occurring.
Example:
/* if page size greater than 1k ... */ -if ( page_length() > 1024 ) + + + + + |
Explanation:
- structure pointer operator ( "->" ) - member operator ( - "." ) - functions and parentheses
It is a general coding practice to put pointers, references, - and function parentheses next to names. With spaces, the - connection between the object and variable/function name is not - as clear.
Example:
a_struct->a_member; +
Instead of: a_struct -> a_member; a_struct . a_member; - function_name (); 4.4.6. Make the last brace of a function stand - outExample:
Instead of: + a_struct -> a_member; a_struct . a_member; function_name (); +
+ 4.4.6. Make the last brace + of a function stand out+ +Example: + +
Instead of: int function1( ... ) { ...code... return( ret_code ); } int - function2( ... ) { } Note: Use 1 blank line before the closing brace and 2 - lines afterward. This makes the end of function standout to - the most casual viewer. Although function comments help - separate functions, this is still a good coding practice. In - fact, I follow these rules when using blocks in "for", "while", - "do" loops, and long if {} statements too. After all whitespace - is free! Status: developer-discretion on the number of blank - lines. Enforced is the end of function comments. 4.4.7. Use 3 character indentionsExplanation: If some use 8 character TABs and some use 3 character TABs, - the code can look *very* ragged. So use 3 character indentions - only. If you like to use TABs, pass your code through a filter - such as "expand -t3" before checking in your code. Example:
Instead + of: + +int function1( ... ) { ...code... return(ret_code); } int + function2( ... ) { } + +Note: Use 1 + blank line before the closing brace and 2 lines afterward. This makes + the end of function standout to the most casual viewer. Although + function comments help separate functions, this is still a good + coding practice. In fact, I follow these rules when using blocks in + "for", "while", "do" loops, and long if {} statements too. After all + whitespace is free! + +Status: + developer-discretion on the number of blank lines. Enforced is the + end of function comments. +
+ 4.4.7. Use 3 character + indentions+ +Explanation: + +If some use 8 character TABs and some use 3 character TABs, the + code can look *very* ragged. So use 3 character indentions only. If + you like to use TABs, pass your code through a filter such as "expand + -t3" before checking in your code. + +Example: + +
4.5. Initializing4.5.1. Initialize all variablesExplanation: Do not assume that the variables declared will not be used - until after they have been assigned a value somewhere else in - the code. Remove the chance of accidentally using an unassigned - variable. Example:
+ 4.5. Initializing+ +
+ 4.5.1. Initialize all + variables+ +Explanation: + +Do not assume that the variables declared will not be used until + after they have been assigned a value somewhere else in the code. + Remove the chance of accidentally using an unassigned variable. + +Example: + +
Note: It is much easier to debug a SIGSEGV if the - message says you are trying to access memory address 00000000 - and not 129FA012; or array_ptr[20] causes a SIGSEV vs. - array_ptr[0]. Status: developer-discretion if and only if the - variable is assigned a value "shortly after" declaration. 4.6. Functions4.6.1. Name functions that return a boolean as a - question.Explanation: Value should be phrased as a question that would logically - be answered as a true or false statement Example:
Note: It is + much easier to debug a SIGSEGV if the message says you are trying to + access memory address 00000000 and not 129FA012; or array_ptr[20] + causes a SIGSEV vs. array_ptr[0]. + +Status: + developer-discretion if and only if the variable is assigned a value + "shortly after" declaration. +
+ 4.6. Functions+ +
+ 4.6.1. Name functions that + return a boolean as a question.+ +Explanation: + +Value should be phrased as a question that would logically be + answered as a true or false statement + +Example: + +
4.6.2. Always specify a return type for a - function.Explanation: The default return for a function is an int. To avoid - ambiguity, create a return for a function when the return has a - purpose, and create a void return type if the function does not - need to return anything. 4.6.3. Minimize function calls when iterating by - using variablesExplanation: It is easy to write the following code, and a clear argument - can be made that the code is easy to understand: Example:
+
+
+ 4.6.2. Always specify a + return type for a function.+ +Explanation: + +The default return for a function is an int. To avoid ambiguity, + create a return for a function when the return has a purpose, and + create a void return type if the function does not need to return + anything. +
+ 4.6.3. Minimize function + calls when iterating by using variables+ +Explanation: + +It is easy to write the following code, and a clear argument can + be made that the code is easy to understand: + +Example: + +
Note: Unfortunately, this makes a function call for - each and every iteration. This increases the overhead in the - program, because the compiler has to look up the function each - time, call it, and return a value. Depending on what occurs in - the block_list_length() call, it might even be creating and - destroying structures with each iteration, even though in each - case it is comparing "cnt" to the same value, over and over. - Remember too - even a call to block_list_length() is a function - call, with the same overhead. Instead of using a function call during the iterations, - assign the value to a variable, and evaluate using the - variable. Example:
Note: + Unfortunately, this makes a function call for each and every + iteration. This increases the overhead in the program, because the + compiler has to look up the function each time, call it, and return a + value. Depending on what occurs in the block_list_length() call, it + might even be creating and destroying structures with each iteration, + even though in each case it is comparing "cnt" to the same value, + over and over. Remember too - even a call to block_list_length() is a + function call, with the same overhead. + +Instead of using a function call during the iterations, assign the + value to a variable, and evaluate using the variable. + +Example: + +
Exceptions: if the value of block_list_length() - *may* change or could *potentially* change, then you must code the - function call in the for/while loop. 4.6.4. Pass and Return by Const ReferenceExplanation: This allows a developer to define a const pointer and call - your function. If your function does not have the const - keyword, we may not be able to use your function. Consider - strcmp, if it were defined as: extern int strcmp( char *s1, - char *s2 ); I could then not use it to compare argv's in main: int main( - int argc, const char *argv[] ) { strcmp( argv[0], "privoxy" - ); } Both these pointers are *const*! If the c runtime library - maintainers do it, we should too. 4.6.5. Pass and Return by ValueExplanation: Most structures cannot fit onto a normal stack entry (i.e. - they are not 4 bytes or less). Aka, a function declaration - like: int load_aclfile( struct client_state csp ) would not work. So, to be consistent, we should declare all - prototypes with "pass by value": int load_aclfile( struct - client_state *csp ) 4.6.6. Names of include filesExplanation: Your include statements should contain the file name without - a path. The path should be listed in the Makefile, using -I as - processor directive to search the indicated paths. An exception - to this would be for some proprietary software that utilizes a - partial path to distinguish their header files from system or - other header files. Example:
Exception:
Note: Please! do not add "-I." to the Makefile - without a _very_ good reason. This duplicates the #include - "file.h" behavior. 4.6.7. Provide multiple inclusion - protectionExplanation: Prevents compiler and linker errors resulting from - redefinition of items. Wrap each header file with the following syntax to prevent - multiple inclusions of the file. Of course, replace PROJECT_H - with your file name, with "." Changed to "_", and make it - uppercase. Example:
Exceptions: + if the value of block_list_length() *may* change or could + *potentially* change, then you must code the function call in the + for/while loop. +
+
+
+ 4.6.4. Pass and Return by + Const Reference+ +Explanation: + +This allows a developer to define a const pointer and call your + function. If your function does not have the const keyword, we may + not be able to use your function. Consider strcmp, if it were defined + as: extern int strcmp(char *s1, char *s2); + +I could then not use it to compare argv's in main: int main(int + argc, const char *argv[]) { strcmp(argv[0], "privoxy"); } + +Both these pointers are *const*! If the c runtime library + maintainers do it, we should too. +
+
+
+ 4.6.5. Pass and Return by + Value+ +Explanation: + +Most structures cannot fit onto a normal stack entry (i.e. they + are not 4 bytes or less). Aka, a function declaration like: int + load_aclfile(struct client_state csp) + +would not work. So, to be consistent, we should declare all + prototypes with "pass by value": int load_aclfile(struct client_state + *csp) +
+
+
+ 4.6.6. Names of include + files+ +Explanation: + +Your include statements should contain the file name without a + path. The path should be listed in the Makefile, using -I as + processor directive to search the indicated paths. An exception to + this would be for some proprietary software that utilizes a partial + path to distinguish their header files from system or other header + files. + +Example: + +
Exception: + +
Note: + Please! do not add "-I." to the Makefile without a _very_ good + reason. This duplicates the #include "file.h" behavior. +
+ 4.6.7. Provide multiple + inclusion protection+ +Explanation: + +Prevents compiler and linker errors resulting from redefinition of + items. + +Wrap each header file with the following syntax to prevent + multiple inclusions of the file. Of course, replace PROJECT_H with + your file name, with "." Changed to "_", and make it uppercase. + +Example: + +
4.6.8. Use `extern "C"` when appropriateExplanation: If our headers are included from C++, they must declare our - functions as `extern "C"`. This has no cost in C, but increases - the potential re-usability of our code. Example:
+ 4.6.8. Use `extern "C"` when + appropriate+ +Explanation: + +If our headers are included from C++, they must declare our + functions as `extern "C"`. This has no cost in C, but increases the + potential re-usability of our code. + +Example: + +
4.6.9. Where Possible, Use Forward Struct - Declaration Instead of IncludesExplanation: Useful in headers that include pointers to other struct's. - Modifications to excess header files may cause needless - compiles. Example:
+ 4.6.9. Where Possible, Use + Forward Struct Declaration Instead of Includes+ +Explanation: + +Useful in headers that include pointers to other struct's. + Modifications to excess header files may cause needless compiles. + +Example: + +
Note: If you declare "file_list xyz;" (without the - pointer), then including the proper header file is necessary. - If you only want to prototype a pointer, however, the header - file is unnecessary. Status: Use with discretion. 4.7. General Coding Practices4.7.1. Turn on warningsExplanation Compiler warnings are meant to help you find bugs. You - should turn on as many as possible. With GCC, the switch is - "-Wall". Try and fix as many warnings as possible. 4.7.2. Provide a default case for all switch - statementsExplanation: What you think is guaranteed is never really guaranteed. The - value that you don't think you need to check is the one that - someday will be passed. So, to protect yourself from the - unknown, always have a default step in a switch statement. Example:
Note: If you + declare "file_list xyz;" (without the pointer), then including the + proper header file is necessary. If you only want to prototype a + pointer, however, the header file is unnecessary. + +Status: Use + with discretion. +
+ 4.7. General Coding + Practices+ +
+
+
+ 4.7.1. Turn on + warnings+ +Explanation + +Compiler warnings are meant to help you find bugs. You should turn + on as many as possible. With GCC, the switch is "-Wall". Try and fix + as many warnings as possible. +
+ 4.7.2. Provide a default + case for all switch statements+ +Explanation: + +What you think is guaranteed is never really guaranteed. The value + that you don't think you need to check is the one that someday will + be passed. So, to protect yourself from the unknown, always have a + default step in a switch statement. + +Example: + +
Note: If you already have a default condition, you - are obviously exempt from this point. Of note, most of the - WIN32 code calls `DefWindowProc' after the switch statement. - This API call *should* be included in a default statement. Another Note: This is not so much a readability issue - as a robust programming issue. The "anomaly code goes here" may - be no more than a print to the STDERR stream (as in - load_config). Or it may really be an abort condition. Status: Programmer discretion is advised. 4.7.3. Try to avoid falling through cases in a - switch statement.Explanation: In general, you will want to have a 'break' statement within - each 'case' of a switch statement. This allows for the code to - be more readable and understandable, and furthermore can - prevent unwanted surprises if someone else later gets creative - and moves the code around. The language allows you to plan the fall through from one - case statement to another simply by omitting the break - statement within the case statement. This feature does have - benefits, but should only be used in rare cases. In general, - use a break statement for each case statement. If you choose to allow fall through, you should comment both - the fact of the fall through and reason why you felt it was - necessary. 4.7.4. Use 'long' or 'short' Instead of - 'int'Explanation: On 32-bit platforms, int usually has the range of long. On - 16-bit platforms, int has the range of short. Status: open-to-debate. In the case of most FSF - projects (including X/GNU-Emacs), there are typedefs to int4, - int8, int16, (or equivalence ... I forget the exact typedefs - now). Should we add these to IJB now that we have a "configure" - script? 4.7.5. Don't mix size_t and other typesExplanation: The type of size_t varies across platforms. Do not make - assumptions about whether it is signed or unsigned, or about - how long it is. Do not compare a size_t against another - variable of a different type (or even against a constant) - without casting one of the values. 4.7.6. Declare each variable and struct on its - own line.Explanation: It can be tempting to declare a series of variables all on - one line. Don't. Example:
Note: If you + already have a default condition, you are obviously exempt from this + point. Of note, most of the WIN32 code calls `DefWindowProc' after + the switch statement. This API call *should* be included in a default + statement. + +Another + Note: This is not so much a readability issue as a robust + programming issue. The "anomaly code goes here" may be no more than a + print to the STDERR stream (as in load_config). Or it may really be + an abort condition. + +Status: + Programmer discretion is advised. +
+
+
+ 4.7.3. Try to avoid falling + through cases in a switch statement.+ +Explanation: + +In general, you will want to have a 'break' statement within each + 'case' of a switch statement. This allows for the code to be more + readable and understandable, and furthermore can prevent unwanted + surprises if someone else later gets creative and moves the code + around. + +The language allows you to plan the fall through from one case + statement to another simply by omitting the break statement within + the case statement. This feature does have benefits, but should only + be used in rare cases. In general, use a break statement for each + case statement. + +If you choose to allow fall through, you should comment both the + fact of the fall through and reason why you felt it was + necessary. +
+
+
+ 4.7.4. Don't mix size_t and + other types+ +Explanation: + +The type of size_t varies across platforms. Do not make + assumptions about whether it is signed or unsigned, or about how long + it is. Do not compare a size_t against another variable of a + different type (or even against a constant) without casting one of + the values. +
+ 4.7.5. Declare each variable + and struct on its own line.+ +Explanation: + +It can be tempting to declare a series of variables all on one + line. Don't. + +Example: + +
Instead of: long a, b, c; Explanation: - there is more room for comments on the - individual variables - easier to add new variables without - messing up the original ones - when searching on a variable to - find its type, there is less clutter to "visually" - eliminate Exceptions: when you want to declare a bunch of loop - variables or other trivial variables; feel free to declare them - on one line. You should, although, provide a good comment on - their functions. Status: developer-discretion. 4.7.7. Use malloc/zalloc sparinglyExplanation: Create a local struct (on the stack) if the variable will - live and die within the context of one function call. Only "malloc" a struct (on the heap) if the variable's life - will extend beyond the context of one function call. Example:
4.7.8. The Programmer Who Uses 'malloc' is - Responsible for Ensuring 'free'Explanation: If you have to "malloc" an instance, you are responsible for - insuring that the instance is `free'd, even if the deallocation - event falls within some other programmer's code. You are also - responsible for ensuring that deletion is timely (i.e. not too - soon, not too late). This is known as "low-coupling" and is a - "good thing (tm)". You may need to offer a - free/unload/destructor type function to accommodate this. Example:
Exceptions: The developer cannot be expected to provide `free'ing - functions for C run-time library functions ... such as - `strdup'. Status: developer-discretion. The "main" use of this - standard is for allocating and freeing data structures (complex - or nested). 4.7.9. Add loaders to the `file_list' structure - and in orderExplanation: I have ordered all of the "blocker" file code to be in alpha - order. It is easier to add/read new blockers when you expect a - certain order. Note: It may appear that the alpha order is broken in - places by POPUP tests coming before PCRS tests. But since - POPUPs can also be referred to as KILLPOPUPs, it is clear that - it should come first. 4.7.10. "Uncertain" new code and/or changes to - existing code, use FIXME or XXXExplanation: If you have enough confidence in new code or confidence in - your changes, but are not *quite* sure of the repercussions, - add this: /* FIXME: this code has a logic error on platform XYZ, * - attempting to fix */ #ifdef PLATFORM ...changed code here... - #endif or: /* FIXME: I think the original author really meant this... - */ ...changed code here... or: /* FIXME: new code that *may* break something else... */ - ...new code here... Note: If you make it clear that this may or may not - be a "good thing (tm)", it will be easier to identify and - include in the project (or conversely exclude from the - project). 4.8. Addendum: Template for files and function - comment blocks:Example for file comments:
Instead + of: + +long a, b, c; + +Explanation: + - there is more room for comments on the individual variables - + easier to add new variables without messing up the original ones - + when searching on a variable to find its type, there is less clutter + to "visually" eliminate + +Exceptions: + when you want to declare a bunch of loop variables or other trivial + variables; feel free to declare them on one line. You should, + although, provide a good comment on their functions. + +Status: + developer-discretion. +
+
+
+ 4.7.6. Use malloc/zalloc + sparingly+ +Explanation: + +Create a local struct (on the stack) if the variable will live and + die within the context of one function call. + +Only "malloc" a struct (on the heap) if the variable's life will + extend beyond the context of one function call. + +Example: + +
+
+
+ 4.7.7. The Programmer Who + Uses 'malloc' is Responsible for Ensuring 'free'+ +Explanation: + +If you have to "malloc" an instance, you are responsible for + insuring that the instance is `free'd, even if the deallocation event + falls within some other programmer's code. You are also responsible + for ensuring that deletion is timely (i.e. not too soon, not too + late). This is known as "low-coupling" and is a "good thing (tm)". + You may need to offer a free/unload/destructor type function to + accommodate this. + +Example: + +
Exceptions: + +The developer cannot be expected to provide `free'ing functions + for C run-time library functions ... such as `strdup'. + +Status: + developer-discretion. The "main" use of this standard is for + allocating and freeing data structures (complex or nested). +
+
+
+ 4.7.8. Add loaders to the + `file_list' structure and in order+ +Explanation: + +I have ordered all of the "blocker" file code to be in alpha + order. It is easier to add/read new blockers when you expect a + certain order. + +Note: It may + appear that the alpha order is broken in places by POPUP tests coming + before PCRS tests. But since POPUPs can also be referred to as + KILLPOPUPs, it is clear that it should come first. +
+
+
+
+ 4.7.9. "Uncertain" new code + and/or changes to existing code, use XXX+ +Explanation: + +If you have enough confidence in new code or confidence in your + changes, but are not *quite* sure of the repercussions, add this: + +/* XXX: this code has a logic error on platform XYZ, * attempting + to fix */ #ifdef PLATFORM ...changed code here... #endif + +or: + +/* XXX: I think the original author really meant this... */ + ...changed code here... + +or: + +/* XXX: new code that *may* break something else... */ ...new code + here... + +Note: If you + make it clear that this may or may not be a "good thing (tm)", it + will be easier to identify and include in the project (or conversely + exclude from the project). +
+
\ No newline at end of file
+ return 0;
+
+}
+
+ 4.8. Addendum: Template for + files and function comment blocks:+ +Example for file + comments: + +
Note: This declares the rcs variables that should be - added to the "show-proxy-args" page. If this is a brand new - creation by you, you are free to change the "Copyright" section - to represent the rights you wish to maintain. Note: The formfeed character that is present right - after the comment flower box is handy for (X|GNU)Emacs users to - skip the verbiage and get to the heart of the code (via - `forward-page' and `backward-page'). Please include it if you - can. Example for file header comments:
Note: This + declares the rcs variables that should be added to the + "show-proxy-args" page. If this is a brand new creation by you, you are + free to change the "Copyright" section to represent the rights you wish + to maintain. + +Note: The + formfeed character that is present right after the comment flower box + is handy for (X|GNU)Emacs users to skip the verbiage and get to the + heart of the code (via `forward-page' and `backward-page'). Please + include it if you can. + +Example for file header + comments: + +
Example for function comments:
Example for function + comments: + +
Note: If we all follow this practice, we should be - able to parse our code to create a "self-documenting" web - page. |
+
Note: If we + all follow this practice, we should be able to parse our code to create + a "self-documenting" web page.
+