[ovs-dev] [PATCH] datapath-windows: Update CodingStyle

[Samuel Ghinet]

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.

---
 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