[ovs-dev] [PATCH 01/15] datapath-windows: Update CodingStyle

Alin Serdean aserdean at cloudbasesolutions.com
Wed Aug 6 19:49:40 UTC 2014


Hi Sam,

Just some pointers from me:
- format the text up to 79 characters.
- " Do not use space after "(" or before ")" ", people using vi would have a harder time
- "o) "_" prefix for private functions", I don't quite understand. Do you mean static?

Alin.


-----Mesaj original-----
De la: dev [mailto:dev-bounces at openvswitch.org] În numele Samuel Ghinet
Trimis: Wednesday, August 6, 2014 7:07 PM
Către: dev at openvswitch.org
Subiect: [ovs-dev] [PATCH 01/15] datapath-windows: Update CodingStyle

Update the file CodingStyle: add more Windows-style rules.

Also, other rules will make code more clear.

Windows Kernel style rules:
o) Type names (structs, enums), constants, symbols, macros.
o) Braces
o) Code annotations
o) Function suffix: Unsafe
o) Switch Cases

Code clarity rules:
o) OVS_ prefixes for types, constants, symbols, macros.
o) "s_" and "g_" prefixes for static and global variables. Normally, Windows style uses "g" instead of
o) g_" and does not have a prefix for static variables.
o) "p" or "pp" prefixes for pointer and pointer-to-pointer variables. This style is used in Windows userspace code style, but not in kernel. Either way, it is useful to distinguish pointer variables from other variable types.
o) "_" prefix for private functions.
o) Component-prefixed function names.

Signed-off-by: Samuel Ghinet <sghinet at cloudbasesolutions.com>
---
 datapath-windows/CodingStyle | 177 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 175 insertions(+), 2 deletions(-)

diff --git a/datapath-windows/CodingStyle b/datapath-windows/CodingStyle index 006adfd..a8f020d 100644
--- a/datapath-windows/CodingStyle
+++ b/datapath-windows/CodingStyle
@@ -42,8 +42,35 @@ guidelines:

   Use upper case to begin the name of a function, enum, file name etc.

-  For types, use all upper case for all letters with words separated by '_'. If
-  camel casing is preferred, use  upper case for the first letter.
+  All types, all enum constants and all symbol definitions must be all upper case.
+If the name is made of multiple words, separate words by "_".
+
+  All macros, symbols, and custom types (i.e. typedef to enums, 
+structs, unions) must be prefixed by "OVS_". Functions must not be prefixed by "Ovs".
+
+  Enum constants must follow the name style of the enum type.
+
+  Example:
+
+typedef enum _OVS_IPPROTO
+{
+    OVS_IPPROTO_ICMP = 1,
+    //constants here.
+};
+
+  This makes easier lookup of enum constants, knowing the enum type.
+
+  All types must be declared with typedef. Type typedef must always be used.
+
+  Example:
+
+typedef struct _OVS_DATAPATH
+{
+    //fields here
+}OVS_DATAPATH, *POVS_DATAPATH;
+
+  Always use OVS_DATAPATH or POVS_DATAPATH. Do not declare variable by  
+ "struct _OVS_DATAPATH dp;"

   It is a common practice to define a pointer type by prefixing the letter  'P' to a data type.  The same practice can be followed here as well.
@@ -72,6 +99,47 @@ OvsDetectTunnelRxPkt(POVS_FORWARDING_CONTEXT ovsFwdCtx,
     return FALSE;
 }

+  Builtin types: UINT, INT, UINT16, VOID etc. are preferred over 
+unsigned int, int, unsigned short, void, etc. Do not use typedef-s for builtin types that are lowercase.
+For example, do not use uint32_t, uint16_t, etc.
+
+  In Visual Studio, ULONG is identical to UINT and with UINT32. The use 
+of ULONG is preferred. Use unsigned types whenever the variable should only have positive values.
+Use typedef-s for UINT32: BE32, BE64, whenever it is known that the variable is Big Endian.
+Use UINT32 only when it is important to know that the value is 32 bit 
+long. Otherwise, use ULONG.
+
+  Variables declared in a .c file that are used only within that .c 
+file must be static and prefixed by "s_" (stands for "static"). Global 
+variables, i.e. variables that are declared in a .h file and used in multiple .c files must be prefixed by "g_" (stands for "global").
+
+  Pointer data types should be prefixed by "p" or "pp".
+
+  Example:
+
+OVS_FLOW* pFlow;
+OVS_FLOW** ppFlow;
+
+  or
+
+POVS_FLOW pFlow;
+POVS_FLOW* ppFlow;
+
+Arrays should be named like this:
+OVS_FLOW* flows;
+OVS_FLOW** pFlows;
+
+  or
+
+POVS_FLOW flows;
+POVS_FLOW* pFlows;
+
+If "*" is preferred in a variable declaration, instead of using 
+P-prefixed type, the "*" must be attached to the type, when possible.
+
+Example:
+
+OVS_FLOW* pFlow; //instead of OVS_FLOW *pFlow;

 COMMENTS

@@ -103,12 +171,41 @@ the name of the function or based on the workflow it is called from.
   In the interest of keeping comments describing functions similar in  structure, use the following template.

+  Code annotations are preferred.
+
 /*
  *----------------------------------------------------------------------------
  * Any description of the function, arguments, return types, assumptions and
  * side effects.
  *----------------------------------------------------------------------------
  */
+
+  All private functions (i.e. functions that are to be used only in one 
+.c file) should be defined as static, and be "_" prefixed.
+
+  Example:
+
+static BOOLEAN _FunctionName(VOID* p)
+{
+    //code
+}
+
+  This makes it clear when studying the code, and when seeing function 
+calls weather the function is supposed to be private or public.
+
+  Public functions that operate on a specific component should have a component name prefix.
+
+  Example:
+
+OVS_FLOW* FlowCreate(VOID* p)
+{
+    //code
+}
+
+  Functions that operate on global data (such as, functions that add / 
+remove flows), and expect the caller to hold a lock beforehand, should 
+be suffixed with "Unsafe". If the component function (e.g. Flow) does 
+not lock its specific lock (e.g. the flow's lock field), but locks any other component's lock or global lock, it should still be suffixed by "Unsafe".

 SOURCE FILES

@@ -137,3 +234,79 @@ quickly figure out where a given module fits into the larger system.

     4. Open vSwitch headers, in alphabetical order.  Use "", not <>,
        to specify Open vSwitch header names.
+
+BRACES
+
+  Use Windows-style braces:
+  The open brace must be on a separate line.
+
+  Example:
+
+if (condition)
+{
+    //code
+}
+
+typedef struct _OVS_DATAPATH
+{
+    //fields here
+}OVS_DATAPATH, *pOVS_DATAPATH;
+
+  All code executed for an if, else, while, do while, or for, must be  
+ enclosed in braces, even if it's only one instruction to be executed.
+  The else part of an if must be on the very next line after the if's 
+ closed  brace (i.e. there should be no blank line in between)
+
+  Example:
+
+if (condition)
+{
+    //code
+}
+else
+{
+  //code
+}
+
+For all other cases than else, after "}", a blank line must follow
+
+SWITCH CASES
+
+  Each "case", if it is preceded by a "break" of a previous case, there 
+must be
+  a blank line in between:
+switch (c)
+{
+    case v1:
+        //code
+        break;
+
+    case v2:
+        //code
+        break;
+}
+
+  Number of lines per file: try to limit to 1000 lines. Maximum allowed should be 1500.
+If a file is grown to large in number of lines, consider splitting it 
+into files, based on the components the functions operate upon.
+
+SPACES WITHIN LINES
+
+  Use spaces between operands and operators.
+
+  Example:
+
+x + y * z
+
+instead of:
+
+x+y*z
+
+  Do not use space after "(" or before ")"
+
+  Example:
+
+(x * y * z) + a
+
+  instead of:
+
+( x * y * z ) + a
--
1.8.3.msysgit.0


_______________________________________________
dev mailing list
dev at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list