Best Practices
From Dcmf
This is to document the little ideas that are conceived but do not require a full design document. These ideas are generallty accepted although they are often not written down.
Contents |
Style Conventions
Indent
If you wish to use indent, we recommend that you use these flags:
indent -gnu --no-tabs --no-space-after-function-call-names
and optionally
--continue-at-parentheses --break-function-decl-args
- --gnu-style -gnu
- -nbad -bap -nbc -bbo -bl -bli2 -bls -ncdb -nce -cp1 -cs -di2 -ndj -nfc1 -nfca -hnl -i2 -ip5 -lp -pcs -nprs -psl -saf -sai -saw -nsc -nsob
Use GNU coding style. This is the default. - --no-tabs -nut
- Use spaces instead of tabs
- --no-space-after-function-call-names -npcs
- Do not put space after the function in function calls
- --continue-at-parentheses -lp
- Line up continued lines at parentheses
- --break-function-decl-args -bfda
- Break the line before all arguments in a declaration
Environment Variables
Standardization
All variables should start with this prefix to ensure consistent access methods:
- Env var prefix : DCMF_*
Assert Alternatives
The simple fact is that asserts reduce performance by introducing branches in the code, however, asserts are needed for debug and development. Please follow these assert recommendations for all DCMF code.
Assert macros
Please use these macros (and only these) when using the former assert() macro in the various communication layers. This will allow us to enable and disable asserts in individual layers to speed up the code performance. These macros will be defined in a short global include file.
- DCMF
- DCMF_abort()
- DCMF_assert(x)
- DCMF_assert_debug(x)
This enables three levels of asserts:
- None - Leaves *_abort() active. These are defined to assert(0) and are used to disable features.
- Shipping/Production - Leaving *_assert() on, but turning *_assert_debug() assert macros off - this will be the standard shipping libraries. Asserts here should be a balance between performance and problem debug ability. Checking for NULL, etc are probably good candidates for *_assert() macros
- Debug - Maximum debug assert() macros turned on. Asserting on almost all conditions that lead to errors can be done with the *assert_debug() macros.
Note: To allow for the effective disabling of these Macros, it is important that people not do work in the macro, such as assignment or function calls. They will be defined to nothing, instead of defining them as (x).
Compile time asserts
The COMPILE_TIME_ASSERT macro is defined in bgp/comm/sys/messaging/messager/Util.h and can be used instead of a runtime assert for certain types of checks. For example, determining if an opaque type is large enough to contain a C++ object.
/**
* \brief Creates a compile error if the condition is false.
*
* This macro must be used within a function for the compiler to process it.
* It is suggested that C++ classes and C files create an inline function
* similar to the following example. The inline function is never used at
* runtime and should be optimized out by the compiler. It exists for the sole
* purpose of moving runtime \c assert calls to compile-time errors.
*
* \code
* static inline void compile_time_assert ()
* {
* // This compile time assert will succeed.
* COMPILE_TIME_ASSERT(sizeof(char) <= sizeof(double));
*
* // This compile time assert will fail.
* COMPILE_TIME_ASSERT(sizeof(double) <= sizeof(char));
* }
* \endcode
*
* Compile time assert errors will look similar to the following:
*
* \code
* foo.h: In function compile_time_assert:
* foo.h:43: error: duplicate case value
* foo.h:43: error: previously used here
* \endcode
*/
#define COMPILE_TIME_ASSERT(expr) switch(0){case 0:case expr:;}
Compiler errors
Switch statements
Error type:
jump to case label crosses initialization of object…
We have been having a few SLES10 specific compile errors come in, and I wanted to discuss the most common variant: an object is "newed" inside a switch statement. Under certain conditions—which tend to happen frequently in the message layer—it is possible to create code that builds without any warnings using a SLES9 toolchain, but causes a fatal error on SLES10.
The following example is some of this code. The problem occurs on the red line (24). It seems that SLES10 gets confused about when that object should be created. If one moves the pointer declaration to the top of func(), it will work fine, and this has been the preferred solution to date. Likewise, if one adds extra braces around the code, the error goes away.
1 #include <stdio.h>
2 #include <new>
3
4 int func(int arg);
5 extern int fork();
6
7 class A
8 {
9 public:
10 A(){bar=1;puts("NEW");};
11 A(int arg){bar=arg;puts("NEW");};
12 int bar;
13 };
14
15 int func(int arg)
16 {
17 char a[sizeof(A)];
18 switch (arg)
19 {
20 case 1:
21 puts("Just one");
22 break;
23 case 2:
24 A *a_ptr = new (a) A();
25 a_ptr->bar = fork();
26 puts("TWO!!!");
27 break;
28 case 3:
29 puts("That's all");
30 break;
31 default:
32 puts("ENOUGH code!");
33 break;
34 }
35 return 0;
36 }
This is the error you should see on the SLES 10 drivers:
/bglhome/jratt/switch.cc: In function #int func(int)#: /bglhome/jratt/switch.cc:28: error: jump to case label /bglhome/jratt/switch.cc:24: error: crosses initialization of #A* a_ptr# /bglhome/jratt/switch.cc:31: error: jump to case label /bglhome/jratt/switch.cc:24: error: crosses initialization of #A* a_ptr#
Moving the pointer declaration to the top is the currently preferred solution
--- /bglhome/jratt/switch.cc 2007-01-03 09:48:49.666937624 -0600
+++ /bglhome/jratt/switch_top.cc 2007-01-03 10:13:18.052858476 -0600
@@ -14,6 +14,7 @@
int func(int arg)
{
+ A *a_ptr;
char a[sizeof(A)];
switch (arg)
{
@@ -21,7 +22,7 @@
puts("Just one");
break;
case 2:
- A *a_ptr = new (a) A();
+ a_ptr = new (a) A();
a_ptr->bar = fork();
puts("TWO!!!");
break;
Another option is to add extra braces:
--- /bglhome/jratt/switch.cc 2007-01-03 09:48:49.666937624 -0600
+++ /bglhome/jratt/switch_brace.cc 2007-01-03 10:13:29.172003760 -0600
@@ -21,9 +21,11 @@
puts("Just one");
break;
case 2:
+ {
A *a_ptr = new (a) A();
a_ptr->bar = fork();
puts("TWO!!!");
+ }
break;
case 3:
puts("That's all");
Missing .cc files
Error type:
undefined reference to vtable for object…
This generally means that the .cc file in which the object is stored didn't get compiled, and thus the .cnk.o file didn't get included into the library. Hopefully, all you have to do is find the missing file.
