GPLTrans Coding Guidelines Mooneer Salem December 7, 2002 Abstract Coding guidelines are needed, regardless of the scope of a software project. This document outlines style and coding guidelines for various files in the GPLTrans directory tree. These guidelines make code maintenence much easier and potentially make code more bug-free and safe. Table of Contents 1 Introduction 2 Recent Changes 3 Style Guidelines 3.1 General 3.1.1 Indentation 3.1.2 Location of Braces 3.1.3 Long Lines 3.1.4 Function Declarations 3.1.5 Comments 3.1.6 Variable Names 4 Coding Standards 4.1 Banned Functions 4.2 Variables 4.3 Documenting Changes 4.4 Code Checking 4.4.1 Hand Verification 4.4.2 Running Splint 4.4.3 Compiling the Code 5 Submitting Patches 5.1 Preparation 5.2 Emailing Patches 5.3 After Submitting 6 Conclusion A Other Sources B Contact Information 1 Introduction It shouldn't be news that software bugs are a big problem. In closed-source software, they are estimated to cost businesses $59.6 billion annually in the U.S alone."Software Bugs Cost U.S. Economy $59.6 Billion Annually" RTI International. http://www.rti.org/page.cfm?objectid=DA7FBFE6-4A4F-4BFD-B77E0FA3C04D9E22. Although this project is an open-source project released under the GNU GPL" GNU General Public License" http://www.gnu.org/licenses/gpl.html, a comprehensive set of coding guidelines is still necessary. For this project, the main priorities of a coding guidelines document is to ensure stability, security and a maintainable code base. By taking simple precautions now, we can avoid needing to reformat source files and fix security holes later. Note: If these guidelines are not followed, your patch or code contribution could be rejected. Please read this document to the letter before proceeding with any patch or other contribution. 2 Recent Changes 12/9/2002 Extra information about the various Splint check levels was added. 12/7/2002 Initial release. 3 Style Guidelines 3.1 General These are general rules that apply to all code files in the source tree, regardless of type. 3.1.1 Indentation The standard for indentation for all source files is one tab per level. Please note this is one tab made with the Tab key on your keyboard, not an arbitrary number of spaces. Every time a new loop or decision construct is begun, another tab should be added to the indent. Once a particular construct is closed, the number of tabs needed per line should be decremented by one. Using this system, the only lines that should be flush with the left hand side are comments, function declarations and preprocessor directives. 3.1.2 Location of Braces Opening braces should be on the same line as the construct that opens it. For cleanliness and simplicity's sake, they should not be directly below the opening construct. 3.1.3 Long Lines If it becomes necessary to write a line that approaches the right side of the editor (such as a long if() statement), then a new line should be made below the beginning of the construct, indenting once until the entire statement's formatted. 3.1.4 Function Declarations All function declarations should be placed in header files that are the same name as the code file they are declared in. If a new header file is created and the functions within it are designed to be used by the rest of the code tree, the header file should be included in gpltrans.h (#include "file.h"). Otherwise, the header should only be included in the source file (file.c). 3.1.5 Comments Comments should be used as much as possible. Since the primary language of this project is C, multiple line comments should be used rather than single-line comments (/* */ as opposed to just //)Although some compilers (notably gcc) support single-line comments in C, their use is not recommended. . At the beginning of every function, there should be a comment block that describes what the function does, the author of it, the input parameters and the return value. For example: /* int test_function(string1, string2) Author: John Doe Purpose: Outputs a test number Input parameters: string1 - a character array representing the string to search string2 - a character array representing the string to search for Return value: 0 if successful, -1 otherwise. */ Comment blocks should also be included after every major block of code within functions that explains what that block does. Contrary to popular belief, it is possible to overdo it, so be careful when adding comments to certain lines of code. 3.1.6 Variable Names Variable names should be descriptive. In this project, variable names will be lowercase if the name is just one word, while they will be a combination of lower and upper case otherwise. For example: int returnValue as opposed to something like int x. There is one exception to this rule, howerver: any variable that will be used as a loop counter may be a single letter. 4 Coding Standards 4.1 Banned Functions There are several functions in the C language that are very unsafe if used. As a result, they are not permitted in any code. Instead, replacements should be used. These are documented below: Insecure Function Replacement ---------------------------------------------------------------------------------------------------------------------------- int strcmp() int strncmp(const char *s1, const char *s2, size_t n) char *strtok() There are third party functions available online that replace this function.none int strcasecmp() int strncasecmp(const char *s1, const char *s2, size_t n) char *tmpnam() int mkstemp(char mkstemp() modifies its argument, so it still might not be a good option.*template) int sprintf() int snprintf(char *str, size_t n, const char *format, ...) int scanf() int sscanf(const char *str, const char *format, ...) 4.2 Variables Variables should be given extra scrutiny. Global variables should be avoided at all costs. Their use will prevent code from being thread-safe and may result in weird interactions due to differing variable scope. Also, in function calls, the return values and the variables passed to functions should be the same variable types. If that is not possible, a cast should be used. 4.3 Documenting Changes After something is changed in the code, an entry should be inserted into the ChangeLog file located in the root of the source tree. This will tell the other developers and anyone else who is interested what changed recently. Doing this also helps in bug tracking. 4.4 Code Checking Before submitting any code to the development team, it should be checked with a program called Splint (formerly known as LCLint)LCLint/Splint can be downloaded at http://lclint.cs.virginia.edu/. . It scans for code which could cause problems. Although the code might still run without taking this step, there could be underlying security vulnerabilities and other problems that may randomly appear at any time. This is the procedure that should be followed when verifying code: 1. Go through the submission by hand and fix any obvious mistakes. 2. Run Splint on the code, noting any errors that appear. 3. Repeat Step 2 until no further errors appear in Splint. 4. Attempt to compile the code. If necessary, fix any compile errors that appear. 5. Once the code is compilable, pass extra parameters to the compiler to emit additional warnings. 4.4.1 Hand Verification When looking through source, careful attention should be paid to how an algorithm is written. If it can be written in a clearer form, please do so. Also, check for anything that uses static character arrays in particular. If anything using static character arrays can be written in a way that uses dynamic memory allocation, it would be in the best interest of security to do so. The fewer that are in the code itself, the fewer chances for a buffer overflow. 4.4.2 Running Splint After a code submission is checked by hand, it should be run through Splint. More than likely, it should output numerous warnings. At this time, extra comments and other code should be added as per the Splint manual to promote further checking (see Appendix A). Eventually, there should be few to no warnings emitted from Splint. However, there may be some warnings which are difficult or almost impossible to remove. It's acceptable to ignore them. Sometimes it may be prudent to start with the weakest checking (splint -weak). This will allow the developer to quickly determine the problems that require immediate review. Once errors are corrected in this mode, splint can be rerun in standard mode (splint -standard). For the purposes of this project, any errors that appear in checks and strict mode (splint -checks and splint -strict, respectively) are not required to be fixed before a patch is sent in, as they generate more useless warnings than anything concreteIt can be inferred from their site that no practical program can run in strict mode without errors! . 4.4.3 Compiling the Code Once all checks have been run, the code should be compiled with the compiler. It's recommended to compile the entire program to ensure any patch or code submission will play nice with the rest of the tree. If no errors appear, the compiler should be run again, but with extra parameters to emit additonal warnings (for instance, -Wall in gcc). Any warnings that appear after adding the extra parameter(s) should be corrected. 5 Submitting Patches 5.1 Preparation Before going any further, make sure the source tree that has been modified is completely clean. (this can be done by running "make distclean" in the source tree) Decompress the original tree into another directory on the system. Then you can produce a patch by running the following: diff -uNr gpltrans-old/ gpltrans/ > patch-20021207.diff replacing gpltrans-old/ with the name of the original source tree, and gpltrans/ with the new source tree.This will ensure that any new files created in the modified source tree will be included when the patch is applied. Note: if the patch is not in this format, it will not be accepted. Patches should be named in the format patch-yyyymmdd.diff, replacing yyyy, mm, and dd with the year, month, and date. Next, compress the patch file itself. It is acceptable to use either gzip or bzip2 to compress the patch file itself. Once that is done, the patch is ready to be submitted. 5.2 Emailing Patches After the patch has been made and compressed, it should be emailed to the development list (see Appendix B for contact information). That way, it can undergo a collective review. Alternatively it can be sent to the developer responsible for the section of code the patch modifies, who can then forward it on to the development list if it is acceptable. The email should have: 1. The name of the patch and the date it was written, 2. A description of what the patch fixes/improves upon, 3. The patch file itself, attached to the email, and 4. What version this patch is to be used against (latest stable or CVS). Note: Not having these four things may delay review of the patch. 5.3 After Submitting After the patch is emailed to the desired people, it will undergo a review process. The developers will look at the patch itself to make sure the changes follow the guidelines laid out in this document. Once that is complete, they will attempt to patch their own copies of the source tree and run tests on it. This will ensure everything still compiles and runs properly. They may wish to email the contributer with further questions. After it is made certain that the patch won't introduce further bugs, it will be accepted and incorporated into the source tree. The contributer will be added to the AUTHORS file, located in the top directory of the source tree. 6 Conclusion It is the sincere hope that this document has been of assistance to anyone who wishes to be a part of the development team. Although this process may seem convulted and complex, it is necessary to ensure code quality. Any comments can be sent to the maintainer of this document. (see Appendix B) A Other Sources * "Secure Programming For Unix and Linux HOWTO" http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/index.html. * "Splint Manual" http://lclint.cs.virginia.edu/manual/. * "Coding Style and Good Computing Practices" http://wizard.ucr.edu/~nagler/coding_style.html. B Contact Information * gpltrans-discuss mailing list (firstname.lastname@example.org) This is the primary contact point for all patch submissions and development conversation. To subscribe, you can visit http://lists.sourceforge.net/mailman/listinfo/gpltrans-discuss. * Mooneer Salem (email@example.com) Lead developer, author of "GPLTrans Coding Guidelines".