Coding standard

From Second Life Wiki
Revision as of 06:11, 13 April 2009 by Boroondas Gupte (talk | contribs) (→‎Linden Variable Types: added missing period)
Jump to navigation Jump to search

Linden Lab Coding Standard

This document defines and formalizes the style of code produced at Linden Lab. Adherence to these conventions is critical to ensuring readable and maintainable code.

Common Guidelines

Comments

When commenting, address the "why" and not the "what" unless the what is difficult to see. If what the code requires commenting, also include why it has to be done that way so maintainers can follow your reasoning. Use complete sentences and correct spelling.

Prefer the use of doxygen compatible comment markup. (See Using Doxygen.)

Sometimes you know something is broken or a bad idea but need to do it anyway -- please follow the special comment token guidelines to not confuse the next engineer. Append your name or initials and the date of the comment when using these special comment tokens.

Special Comment Tokens
Token Meaning Deprecated tokens
*FIX The associated code is actually broken and has known failure cases. All instances of these should be fixed before shipping. Do not use this tag for hacks or suggested development. Do not simply tag the code with the token alone or simply provide instructions since then it is unclear if that is how it is broken. For example: "// *FIX: invalidate stored textures when number of faces change" -- is really unclear if it is an instruction or a buggy side effect of the code segment. FIXME BROKEN BUG
*HACK The associated code relies on external information or processes outside the normal control flow for the function to work. HACK
*TODO The associated code should be optimized, clarified, or made more reliable. Provide instructions along with why it is probably a good idea to execute on the comment. TODO
*NOTE This denotes that there is something tricky or special about the associated code above and beyond answering the typical 'what' and 'why.' NOTE "NOTA BENE" NB

Examples:

// *FIX: This will fail if the packets arrive out of order.

// *HACK: Filter out some extra information from this 
// packet and put it into the status bar since it is more 
// up to date than the last balance update.

// *TODO: This call does not always need to be made
// and is computationally expensive. Write a test to see
// if it must be made during this iteration.

// *NOTE: The tile is actually 257 pixels wide, so
// we have to fudge the texture coordinates a bit for this
// to look right.

No New Serialization Formats

After much effort and unit-testing, LLSD is now the sanctioned format for all new serialization tasks. Do not implement another serialization scheme unless you have a signed and notarized letter from Elvis explicitly stating that a new serialization format is a good idea. If speed is essential, LLSD has a fairly speedy binary parser and formatter available in c++ and python.

Unicode

Use UTF-8 for all string serialization and communication between processes. Do not use UTF-16 except when interfacing with Win32.

File Names

File names should be significant and of reasonable length, with no spaces or uppercase letters. Separate words in the name can be separated with the underline character.

Dead Code

Do not leave commented-out code in any commits that you make. Delete that code instead.

Block-commented out code is especially problematic because it is not distinguishable from live code in a grep. Our version control systems are good enough that any code that was checked in is retrievable even after deletion.

c++

Source Files

All c++ source code files should begin with the header shown below:

/** 
 * @file filename
 * @author Optional author field
 * @date Optional iso8601 creation date
 * @brief brief description of the file
 *
 * $LicenseInfo:firstyear=2008&license=internal$
 * Copyright (c) 2008, Linden Research, Inc.
 * $/LicenseInfo$
 */

The $LicenseInfo$ section is critically important so that we can process the source files based on license agreements. If you're unsure, use internal. The licenses we support are:

  • internal - something probably not meant for distribution unless we have an agreement with the third party.
  • viewergpl - the viewer and linden libraries distributed with it
  • mit - a basic mit license for some libraries such as mulib, eventlet, and indra.ipc.

Replace the 2008 with the year the file is created. post-processing can convert the firstyear and license parameters into a binding copyright and license notice. The linden/scripts/apply-license.py script will do do that process on any collection of files passed on the command line. The script linden/scripts/insert-copyright.py will create or prepend appropriate license and copyright information for python and c source files.

All C++ source files should end in a newline. If not, gcc will fail with an error, as it enforces the C++ standard which requires one.

Headers should be included in the following order:

// All files MUST include one of the following first: 
//  (includes linden_preprocessor.h, stdtypes.h, and some common headers)
#include "llviewerprecompiledheaders.h"	// (NEWVIEW)
// OR
#include "llsimprecompiledheaders.h" // NEWSIM
// OR
#include "linden_common.h" // the ll libraries

#include "llfoo.h"		// Associated header, should have no hidden dependencies

#include <map>			// stl headers
#include <math.h>		// std headers
#include "vorbis/ogg.h"		// External library headers

#include "llbar.h"		// Other linden headers

File Names

All c++ source files should be prefixed with 'll'.

File names must end with the appropriate suffix:

  • cpp for ANSI C++ code
  • h for included declarations
  • inl for large chunks of inlined c++ code. Avoid this.
  • asm for assembly files. Do not do this without a note from some deity forgiving your transgressions.

Include Files

Include files should be specified using relative paths using the '/' character to help ensure portability. In the code relative paths should not be used. Instead, paths should be indicated in the Makefile or VC++ project file.

Compiler directives used to prevent multiple header file inclusion should be based on the file name with a "LL_" preface. Include dependent headers in the header file, but use forward declarations where possible.

For example:

#ifndef LL_LLFOO_H
#define LL_LLFOO_H

#include "llbar.h"

class LLReferencedData; // forward declaration (no need to include "llrefrenceddata.h")

class LLFoo : public LLBar
{
public:
    LLFoo();
    void setData(LLReferencedData& refdata);
private:
    LLReferencedData* mRefData;
};

#endif //LL_LLFOO_H

Basic Facilities

Linden Variable Types

The following basic types are defined in "stdtypes.h" and should be used to ensure clarity and cross platform functionality:

  • Integral Types
    • The first letter specifies 'S' for signed integers and 'U' for unsigned integers
    • The type ends with the number of bits used to represent the integer
    • Examples: S8 (signed 8-bit integer), U32 (unsigned 32 bit integer)
    • The integer types are: U8, S8, U16, S16, U32, S32, U64, S64
    • The char type may used when required by defualt C functions that need a char
  • Floating Point Types
    • F32 denotes a 32-bit floating point value ("float")
    • F64 denotes a 64-bit floating point value ("double")
  • Other Types
    • Prefer the use of the standard c++ bool. This is especially for comparators like operator< (using bool instead of BOOL will prevent lots of warnings when using STL). BOOL denotes a 32-bit integer value that is used to denote TRUE or FALSE. It is specifically not bool to prevent mistakes caused by "sizeof(bool)" being implementation specific.

Project Defines

  • Use LL_WINDOWS, LL_LINUX, and LL_DARWIN for platform specific code.
  • Use LL_RELEASE and LL_DEBUG for release and debug specific code.
  • LL_RELEASE_FOR_DOWNLOAD and LL_RELEASE are both defined in versions destined to be in a resident's hands.
  • Prefer testing for the positive when using #if, and use #if/#if ! instead of #ifdef/#ifndef. For example:
#if LL_DEBUG && !LL_DARWIN

instead of:

#ifndef LL_RELEASE && !defined(LL_DARWIN)

However, this construct is still useful:

#ifndef TRUE
#define TRUE 1
#endif

Usage

String Handling

See C++ Strings for more guidelines on strings in the C++ codebase.

Use std::string as a string container and std::ostringstream for constructing strings from multiple sources.

If you must use printf style formatting, use snprintf() or llformat(). Never use user supplied data (including configuration files) to specify formatting instructions to those functions.

Avoid using c-string libraries. If you are writing something where speed is critical, prefer the use of std::vector<char> since it's usage is less error prone, you can explicitly allocate or occupy memory, it is easy to null terminate with a single call to container.push_back('\0'), and it is char* compatible with a call to &container[0].

The following c functions are forbidden:

  • sprintf() and vsprintf() -- most of the printf family of functions are like individual invitations to hackers to overflow a buffer. Don't use them.
  • strcpy() and strncpy() -- both of these functions are dangerous and difficult to use correctly. For example, did you know that strncpy() does not null terminate if no null is found in the source?
  • strcat() and strncat() -- these methods are inefficient in comparison to using ostringstream() and are easy to misuse. The strncat() function is almost forgivable, but please just avoid these.
  • gets() -- Never, ever use this function.

Our legacy being what it is, there are some unavoidable cases where we are forced to use c-string functions. Here are some tips:

  • sscanf(), fscanf(), etc... -- For these methods, always supply a maximum width for strings. If you do not know how to do this off the top of your head, do not use these functions. Even if you think you know, read the man page again.
  • snprintf(), fprintf(), vsnprintf(), etc.. -- As noted above, never let anything other than the program determine the formatting. Never trust configuration files, assets, message system data, etc.

Unicode

Useful functions for dealing with UTF-8 can be found in llstring.h. Remember that truncating a UTF-8 string must be done on glyph rather than byte boundaries or your string may no longer be valid UTF-8. Many methods in std::string and LLString are not UTF-8 safe since they work on sizeof(char_t) byte boundaries. Similarly, the functions in that header file which do not contain 'ut8f' in the prototype are probably not UTF-8 safe.

Do not use UTF-16 except with interfacing with Win32.

Avoid the use of wide strings except when iteration speed is essential, or you are interfacing with the host operating system.

Containers

Prefer the use of the containers available in the c++ standard template library. They are well tested and in common usage across the industry -- thus, nobody has to learn the special (and lame) quirks of containers like LLMap<> and LLLinkedList<>.

This is one of the few places where specialized implementations can yield significant memory or speed improvements. Do not create those containers until you have profiling results that prove the container is the bottleneck.

Never use std::vector<bool>.

Memory Management

Use new and delete (and delete[] ) for your memory management. Don’t use malloc() and free().

Never store a pointer as object member data unless it is that object's job to delete the pointer. This means pointers should usually only be held in smart-pointer objects, as private member data allocated and deleted in the same cpp file, or in management specialization classes like LLRoundRobin. Never save a pointer extracted from a smart-pointer past the boundary of a stack frame. Always use handles provided by specialized containers when keeping a reference past a stack frame.

Remember that C++ defines delete such that delete NULL doesn’t crash or cause errors. Thus, you don’t need this pattern:

 
if (foo)			// unnecessary if(), delete works just fine on NULL pointers
{
	delete foo;
	foo = NULL;
}

Memory Usage Tracking

When using any memory allocation function - E.g. new, STL push_back in vectors, insert in an STL map, you need to preset a variable for memory tracking purposes. We have a build mode that allows us to track all memory allocation, but it does require that a memory allocation is preset to a type before the memory allocation takes place. E.g.

if (!foo)
{
       LLMemType mt1(LLMemType::MTYPE_DRAWABLE);
       foo = new LLFooType;
}

The crucial part is the LLMemType mtw(LLMemType::MTYPE_APPROPRIATE_TYPE) - this presets the memory tracking system so it knows what kind of allocation is about to happen, so it can track it correctly.

All the current types you can assign are defined wiht LLCommon/llmemtype.h. If you need a new one, just insert it and be sure you update the text of what that type is inside of llMemoryView.cpp.

NOTE - you do NOT need to put this tracking line in where memory is Deallocated - you don't need it around delete, STL erase etc. The internal tracking system keeps track of what the memory is so when it's deleted you don't have to tell it what the memory was used for.

Exception Handling

Exceptions have been recently enabled for all platforms and targets in the main indra code base. If you are familiar with their use, you can use them judiciously in the code. Be sure to make sure the places where you are throwing exceptions will not cause resource leaks, and be sure to comment what linden exceptions might be thrown with the function declaration.

  • do not use exception specifications
  • do not throw in a destructor (ever)
  • read up on exceptions before using them

Resources

Error Messsages

  • See Logging_System_Overview for detailed info on using the runtime logging system. (Test plan for logging system found at: Configurable Error Logging Test)
  • Do not wrap your messages in #ifdef _DEBUG, unless it is inside a performance critical loop. The logging constructs compile to very small amounts of code, and are very efficiently skipped at run time. And the ability to turn on your debug messages at run-time, even in a deployed release build, will make you very happy one day.

Obvious Things We All Know

  • No magic numbers
  • Use TRUE and FALSE when working with BOOL (but prefer "bool" in general)
  • Use NULL for pointer compares to 0
  • Use '\0' for character compares to 0
  • Use const and inline rather than #define whenever possible. Prefer optimizing _after_ you have proven that the code you want to inline actually has a significant benefit from in-lineing.
  • Remember that C++ does not have garbage collection
  • Comment non-intuitive member variables
  • Fix all compiler warnings.
  • Take advantage of the fact that gcc generates different, sometimes better, warnings than Visual Studio.
  • Don't use assignments in while loops, eg while (foo = nextData()) as this causes gcc to emit warnings. Use while ((foo = nextData()) != NULL) or restructure the code.

Style

Naming Convention

The following name conventions apply to non-iterators:

  • Names should include one or more English words that are relevant to the entity being named. Try to use non-programming words, like "EmployeeList" rather than "LinkedListOfStrings"
  • C functions are in lowercase, eg main(), update_world_time()
  • Local variables and parameters are in lowercase, eg frame_count
    • Local variables are not camel-case. thisIsNotALocalVariable
  • Class methods start with a lowercase character but are otherwise CamelCase, eg destroyWorld()
  • Non-static class member variables start with an 'm' followed by an uppercase character, eg mCameraPosition
  • Static class member variables start with an 's' followed by an uppercase character, eg sInstance
  • Structures and Classes start with the uppercase 'LL', followed by the name with an uppercase first letter, eg LLMyClass
  • Enums start with the uppercase 'E', followed by the name with an uppercase first letter, eg EJerkyType. The enum values themselves should use the initials of the type, e.g. JT_TURKEY, JT_BEEF, etc.
  • Variables are in MKS (meters, kilograms, seconds) unless otherwise specified, e.g. frame_time is seconds, frame_time_ms is milliseconds.
  • Data sizes are bytes unless otherwise specified, e.g. size vs. size_bits.
  • Global variables start with a lowercase 'g', followed by the name with an uppercase first letter, eg gGlobalTimeCounter
  • Compiler preprocessor directives should start with LL_ and consist of all uppercase letters separated with underscores, eg LL_DEGREES_TO_RADIANS()
  • Externally available const global variables that replace #defines should start with LL_ and consist of all uppercase letters separated with underscores, eg LL_SECONDS_IN_DAY
  • File scoped const global variables that replace #defines need only consist of all uppercase letters separated with underscores, eg SECONDS_BETWEEN_CLEANUP
  • You may optionally append a suffix character signifying the type, for example, fontp is a font pointer, mTimeInSecondsf is a float

Enums

Enums generally are declared using this form:

typedef enum e_new_enum
{
  NE_ALPHA,
  NE_BETA,
  NE_GAMMA
} ENewEnum;

Class Structure

Member functions come first, followed by member variables at the end. Write it assuming a time-pressured programmer is going to read it from top to bottom looking for something they can use, so put the exported functions at the top, eg:

class LLMyClass	
{
public:
	LLMyClass();
	~LLMyClass();

	void		init(S32 area, F32 priority);

	void		setVariable(BOOL variable)	{ mVariable = variable; }
	virtual void	draw();

private:
	BOOL		mVariable;
        static U32      sCount;
	etc.
};
  • Optionally, one line functions may be written in the class declaration header. Functions inlined for speed should go in the header. Other functions should go in the .cpp file.
    • NOTE: Functions inlined this way do not require the 'inline' compiler hint
    • NOTE: virtual functions can not actually be inlined by the compiler so they should reside in the .cpp file because
      • a) otherwise the linker has to resolve each instance into a single function
      • b) it is misleading to imply that the function will be inlined when it will not be (e.g. don't use virtual functions inside performance critical inner loops)
  • Make sure that you initialize ALL member variables prior to use (often easiest to use an init() member function).

Member Visibility

Class declarations represent contracts and all non-public members are promises. It's important to promise as little as possible because it's much easier to expose more functionality later than it is to take it back. It also requires less precious time from each programmer attempting to use a given class when the non-public members are kept to a minimum. If you see something that's private, you know that you can safely ignore it, otherwise the designer is signaling that it is something that might potentially want to use. Therefore please use these guidelines to keep visibility to a minimum, and remember that everything that applies to data members also applies to typedef, enum, class and method members.

  • Prefer protected members to public, and prefer private members to both.
  • Prefer private inner classes to friends.
  • Document all non-private members but only document implementation details in the .cpp file.
  • Avoid creating a member in the first place when not absolutely needed. E.G. implement getNumPixels() { "return mWidth * mHeight; } rather than getNumPixels() { return mArea; } and having to always make sure that mArea is kept in sync with the current width and height. Remember: All class members are global to the class and all standard wisdom regarding globals apply.
  • Prefer accessor methods to accessible data.
  • Prefer local variables to member variables. Sometimes people create member variables as a way to share data between methods. When possible, it's better to create a local variable in one method and pass it as an argument to the ones that needed it.
  • Declare local variables as close to their point of use as possible. I.E. within nested blocks and as far down within its block as possible.

Static Members

Use static class members for global variables and singletons. Since these are globals, avoid using statics when practical. Avoid static class instances which will be globally constructed, use pointers and construct them in an initializer instead.

class LLFoo
{
    static LLFoo* sInstance; // Don't do this -> static LLFoo sInstance;
    static void initClass();
};
...
//static
LLFoo* LLFoo::sInstance = NULL;

//static
void LLFoo::initClass()
{
    sInstance = new LLFoo();
}

Indentation

Use 4 character tabs for indentation and ensure that your editor is not converting tabs to spaces. When possible, use indentation to clarify variable lists and Boolean operations.

Braces

Source code should be written using braces on a line by themselves, similar to the gnu standard (not the more compact k&r standard), i.e.

 
while (foo < 10)
{
}

Instead of:

while (foo < 10) {
}

And:

do 
{
	do_stuff();
} while (foo < 10);

Instead of:

do {
	do_stuff();
}
while (foo < 10);

And:

 
if (foo < 10)
{
	do_stuff();
}
else
{
	do_stuff();
}

Instead of:

if (foo < 10) {
	do_stuff();
} else {
	do_stuff();
}

Use braces for clarity, even when not strictly required, e.g.

for (j = 0; j < NUM_TEXTURES; ++j)
{
	do_stuff();
}

Comments

Use C++ style comments for single line comments and member variable descriptions. Use classic C style comments /* */ for temporarily commenting out large sections of code.

Function Declaration

The return value of the function should be on the same line as the function name, e.g.

void foo_func(S32 bar)
{
	// do stuff
}