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
(gpltrans-hide@address.com)
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 (hide@address.com)
Lead developer, author of "GPLTrans Coding Guidelines".