Re: [dpdk-dev] tools brainstorming

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Apr 08, 2015 at 02:16:55PM +0000, Wiles, Keith wrote:
> 
> 
> On 4/8/15, 5:43 AM, "Butler, Siobhan A" <siobhan.a.butler@intel.com> wrote:
> 
> >Hi all,
> >To add to the tools brainstorming - I propose we use the following Coding
> >Standards as the basis of guidelines on coding style going forward.
> >The style outlined below is in alignment with the current convention used
> >for the majority of the project.
> >Any thoughts/suggestions or feedback welcome.
> >Thanks
> >Siobhan :)
> ><siobhan.a.butler@intel.com>
> >
> >
> >
> >Coding Style
> >~~~~~~~~~~
> >
> >Description
> >-----------
> >
> >This document specifies the preferred style for source files in the DPDK
> >source tree. 
> >It is based on the Linux Kernel coding guidelines and the FreeBSD 7.2
> >Kernel Developer's Manual (see man style(9)),
> >but was heavily modified for the needs of the DPDK. Many of the style
> >rules are implicit in the examples.
> >Be careful to check the examples before assuming that style is silent on
> >an issue. 
> >
> >General Guidelines
> >------------------
> >
> >The rules and guidelines given in this document cannot cover every
> >situation, so the following general guidelines should be used as a
> >fallback: 
> >The code style should be consistent within each individual file, and
> >within each file in a given directory or module - in the case of creating
> >new files 
> >The primary reason for coding standards is to increase code readability
> >and comprehensibility, therefore always use whatever option will make the
> >code easiest to read.
> >
> >The following more specific recommendations apply to all sections, both
> >for C and assembly code:
> >Line length is recommended to be not more than 80 characters, including
> >comments. [Tab stop size should be assumed to be at least 4-characters
> >wide] 
> >Indentation should be to no more than 3 levels deep.
> >NOTE The above are recommendations, and not hard limits. However, it is
> >expected that the recommendations should be followed in all but the
> >rarest situations.
> >C Comment Style
> >
> >Usual Comments
> >--------------
> >
> >These comments should be used in normal cases. To document a public API,
> >a doxygen-like format must be used: refer to Doxygen Documentation.
> > /*
> >  * VERY important single-line comments look like this.
> >  */
> > 
> > /* Most single-line comments look like this. */
> > 
> > /*
> >  * Multi-line comments look like this.  Make them real sentences. Fill
> >  * them so they look like real paragraphs.
> >  */
> 
> Did you mean to have the Œ*¹ aligned, if so good, if not then it does not
> make sense to not align them. The indent of one space here does not help
> convey any information IMO.
> >
> >License Header
> >--------------
> >
> >Each file should begin with a special comment tag which will contain the
> >appropriate copyright and license for the file (Generally BSD License).
> >After any copyright header, a blank line should be left before any other
> >contents, e.g. include statements in a C file.
> >
> >C Preprocessor Directives
> >-------------------------
> >
> >Header Includes
> >
> >In DPDK sources, the include files should be ordered as following:
> > libc includes (system includes first)
> > DPDK EAL includes
> > DPDK misc libraries includes
> > application-specific includes
> >
> >Example: 
> > #include <stdio.h>
> > #include <stdlib.h>
> > 
> > #include <rte_eal.h>
> > 
> > #include <rte_ring.h>
> > #include <rte_mempool.h>
> > 
> > #include "application.h"
> >
> >
> >Global pathnames are defined in <paths.h>. Pathnames local to the program
> >go in "pathnames.h" in the local directory.
> > #include <paths.h>
> >
> >
> >Leave another blank line before the user include files.
> > #include "pathnames.h"         /* Local includes in double quotes. */
> >
> >NOTE Please avoid, as much as possible, including headers from other
> >headers file. Doing so should be properly explained and justified.
> >Headers should be protected against multiple inclusion with the usual:
> > #ifndef _FILE_H_
> > #define _FILE_H_
> > 
> > /* Code */
> > 
> > #endif /* _FILE_H_ */
> >
> >
> >Macros
> >
> >Do not ``#define`` or declare names in the implementation namespace
> >except for implementing application interfaces.
> >
> >The names of ``unsafe`` macros (ones that have side effects), and the
> >names of macros for manifest constants, are all in uppercase.
> >
> >The expansions of expression-like macros are either a single token or
> >have outer parentheses. If a macro is an inline expansion of a function,
> >the function name is all in lowercase and the macro has the same name all
> >in uppercase. Right-justify the backslashes;
> >it makes it easier to read. If the macro encapsulates a compound
> >statement, enclose it in a do loop, so that it can be used safely in if
> >statements. 
> >Any final statement-terminating semicolon should be supplied by the macro
> >invocation rather than the macro, to make parsing easier for
> >pretty-printers and editors.
> > #define MACRO(x, y) do {                                        \
> >         variable = (x) + (y);                                   \
> >         (y) += 2;                                               \
> > }while (0)
> 
> I have seen this  Œwhile( /*CONSTCOND*/0 )¹ (I think that is the comment)
> plus I have see this Œwhile((0))¹ as gcc complained about a constant in
> the while(). This one maybe fixed at this point in the compilers.

I've never seen a compiler error complaining about a do { } while (0), since this
is the generally recommended way to do define code blocks in macros.

> >
> >NOTE Wherever possible, enums and typedefs should be preferred to macros,
> >since they provide additional degrees
> >of type-safety and can allow compilers to emit extra warnings about
> >unsafe code. 
> >
> >Conditional Compilation
> >-----------------------
> >
> >When code is conditionally compiled using #ifdef or #if, a comment may be
> >added following the matching #endif or #else to
> >permit the reader to easily discern where conditionally compiled code
> >regions end. This comment should be used only for
> >(subjectively) long regions, regions greater than 20 lines, or where a
> >series of nested #ifdef 's may be confusing to the reader.
> >Exceptions may be made for cases where code is conditionally not compiled
> >for the purposes of lint(1), even though the uncompiled
> >region may be small. The comment should be separated from the #endif or
> >#else by a single space. For short conditionally compiled regions,
> >a closing comment should not be used.
> >
> >The comment for #endif should match the expression used in the
> >corresponding #if or #ifdef. The comment for #else and #elif
> >should match the inverse of the expression(s) used in the preceding #if
> >and/or #elif statements. In the comments,
> >the subexpression defined(FOO) is abbreviated as FOO. For the purposes of
> >comments, #ifndef FOO is treated as #if !defined(FOO).
> > #ifdef KTRACE
> > #include <sys/ktrace.h>
> > #endif
> > 
> > #ifdef COMPAT_43
> > /* A large region here, or other conditional code. */
> > #else /* !COMPAT_43 */
> > /* Or here. */
> > #endif /* COMPAT_43 */
> > 
> > #ifndef COMPAT_43
> > /* Yet another large region here, or other conditional code. */
> > #else /* COMPAT_43 */
> > /* Or here. */
> > #endif /* !COMPAT_43 */
> >
> >NOTE Conditional compilation should be used only when absolutely
> >necessary, as it increases the number of target binaries that need to be
> >built and tested. 
> >C Types
> >
> >Integers
> >
> >For fixed/minimum-size integer values, the project uses the form uintXX_t
> >(from stdint.h) instead of older BSD-style integer identifiers of the
> >form u_intXX_t. 
> >
> >Enumerations
> >------------
> >
> >Enumeration values are all uppercase.
> > enum enumtype { ONE, TWO } et;
> >
> >
> >Bitfields
> >---------
> >
> >The developer should group bitfields that are included in the same
> >integer, as follows:
> > struct grehdr {
> >   uint16_t rec:3,
> >       srr:1,
> >       seq:1,
> >       key:1,
> >       routing:1,
> >       csum:1,
> >       version:3,
> >       reserved:4,
> >       ack:1;
> > /* ... */
> > }
> 
> I like to line up the Œ:¹ just to make it a bit more readable, but I can
> live without it.
> >
> >
> >Variable Declarations
> >---------------------
> >
> >In declarations, do not put any whitespace between asterisks and adjacent
> >tokens, except for tokens that are identifiers related to types.
> 
> Much more readable to have the space between pointer and token IMO.
> 

This is not the way things are done right now in our code, and I see no reason
to change it. [Does checkpatch complain if you insert spaces?]

> > 
> >(These identifiers are the names of basic types, type qualifiers, and
> >typedef-names other than the one being declared.)
> >Separate these identifiers from asterisks using a single space.
> 
> Not sure I understand here, can you give an example?

const int *x; /* no space between asterisk and variable name token */
const int * const x; /* space between asterisk and keyword */

> 
> > 
> >
> >Structure Declarations
> >
> >When declaring variables in structures, declare them sorted by use, then
> >by size (largest to smallest), and then in alphabetical order.
> >Alignment constraints may override the previous rules. The first category
> >normally does not apply, but there are exceptions.
> >Each structure element gets its own line. Try to make the structure
> >readable by aligning the member names using spaces as shown below.
> >Names following extremely long types, which therefore cannot be easily
> >aligned with the rest, should be separated by a single space.
> 
> Sorting variables in structures and other places just seem to much
> trouble. Sometimes you want to group variables together as they are used
> together in the code below.

Hence the "sorted by use" - where appropriate.

> 
> Ordering structure members is not going work as alignment and grouping is
> more important. Alignment is important as we all know holes in structures
> which can cause some very odd bugs from arch to arch. Also order of the
> members to a structure could be dependent on hardware or some other
> overlay structure being used to reference that structure. The comments
> should point this issue out in case someone decides to change the order.
> My main concern is trying to keep up with fact a set of members in one
> structure are sorted and in another they are not for some reason.

Not sure I understand what the issue is here. Where "by use" does not apply,
i.e. in most cases, we hope, we group by size from largest to smallest, thereby
avoiding wasting space with packing i.e. holes. Overall, I think the above
guideline is pretty reasonable.

> Sorting local variables is also not reasonable, for grouping of variables
> and being able to maintain the order.
> 
> > 
> > struct foo {
> >         struct foo      *next;          /* List of active foo. */
> >         struct mumble   amumble;        /* Comment for mumble. */
> >         int             bar;            /* Try to align the comments. */
> >         struct verylongtypename *baz;   /* Won't fit with other members
> >*/
> > };
> >
> >
> >Major structures should be declared at the top of the file in which they
> >are used, or in separate header files if they are used
> >in multiple source files. Use of the structures should be by separate
> >declarations and should be extern if they are declared in a header file.
> >
> >Queues
> >
> >Use queue(3) macros rather than rolling your own lists, whenever
> >possible. Thus, the previous example would be better written:
> > #include <sys/queue.h>
> > 
> > struct foo {
> >         LIST_ENTRY(foo) link;      /* Use queue macros for foo lists. */
> >         struct mumble   amumble;   /* Comment for mumble. */
> >         int             bar;       /* Try to align the comments. */
> >         struct verylongtypename *baz;   /* Won't fit with other members
> >*/
> > };
> > LIST_HEAD(, foo) foohead;          /* Head of global foo list. */
> >
> >
> >DPDK also provides an optimized way to store elements in lockless rings.
> >This should be used in all data-path code, when there are several
> >consumer and/or producers to avoid locking for concurrent access.
> >
> >Typedefs
> >
> >Avoid using typedefs for structure types. For example, use:
> > struct my_struct_type {
> > /* ... */
> > };
> > 
> > struct my_struct_type my_var;
> >
> >
> >rather than: 
> > typedef struct my_struct_type {
> > /* ... */
> > } my_struct_type;
> > 
> > my_struct_type my_var
> >
> >
> >Typedefs are problematic because they do not properly hide their
> >underlying type; for example, you need to know if the typedef is
> >the structure itself, as shown above, or a pointer to the structure. In
> >addition, they must be declared exactly once, whereas an
> >incomplete structure type can be mentioned as many times as necessary.
> >Typedefs are difficult to use in stand-alone header files.
> 
> The use of typedefs is not problematic IMO as they do provide a bit
> shorter text, but they allow defining a clean method to define new types.
> The problem I see in the description here is we do not use the _t to
> denote a typedef type and we should when defining a typedef. If we define
> the following:
> 
> typedef struct my_struct_s {
>     /* Š */
> } my_struct_t;
> 
> Do not do the following:
> 
> struct my_struct {
>     /* Š */
> };
> typedef struct my_struct   my_struct_t;
> 
> I like to use the _s for the structure tag name to denote it is the
> non-typedef name and use the _t to denote the typedef. Not having the _s
> is OK, but I would prefer the _s format. Splitting up the struct define
> and typedef is not a problem, but some like to define all of the
> structures at the top and then list all of the typedefs later. I prefer
> them together to make the code more readable.
> 

DPDK does not currently use typedefs, except for function pointer types, which are
more awkward than normal to deal with. I see no real reason to start using them,
as I believe the objections to their use given in the proposed document are
still valid.
I also think having to resort to prefixes is ugly, and if we did start using
typedefs with _t prefixes in place of structures, we end up with very different
looking code meaning either:
* we have a codebase with two very different look and feels, or
* we end up with major rework to redo all code to the new style.

> >The header that defines the typedef must be included before the header
> >that uses it, or by the header that uses it (which causes namespace
> >pollution),
> 
> Namespace pollution, I do not fully understand the concern here.

You should not have to include a whole header file just to get a particular 
structure definition that you use just a pointer to. Doing so, just fills up
your namespace with all the other definitions in the header file you don't need.
Better instead to just put in a single line saying e.g.:

struct rte_mbuf;

> > 
> >or there must be a back-door mechanism for obtaining the typedef.
> 
> As for the ordering problem of typedefs and when they get included we
> should attempt to order the headers correctly, but in some odd cases the
> use of the Œstruct my_struct_s¹ instead of the typedef is OK, but we
> should try to fix the ordering problem. Today we have Œstruct my_struct¹
> to forward declare structures when the header is not included, which seems
> to be a back-door solution, correct?
> 

Or we can just continue to not use typedefs, in which case there is no issue :-)

> 
> > 
> >NOTE #defines used instead of typedefs also are problematic (since they
> >do not propagate the pointer type correctly due to direct text
> >replacement). 
> >For example, ``#define pint int *`` does not work as expected, while
> >``typedef int *pint`` does work. As stated when discussing macros,
> >typedefs 
> >should be preferred to macros in cases like this.
> >When convention requires a typedef; make its name match the struct tag.
> >Avoid typedefs ending in ``_t``, except as specified in Standard C or by
> >POSIX. 
> > /* Make the structure name match the typedef. */
> > typedef struct bar {
> >         int     level;
> > } BAR;
> > typedef int             foo;            /* This is foo. */
> > typedef const long      baz;            /* This is baz. */
> 
> This one does not make sense the ŒBAR¹ to me looks like a MACRO not a
> typedef, this is why using the Œbar_t' solution makes the most sense.
> 
> One more item having the pointer type in the typedef, which is a to me
> hiding the pointer in the typedef and should be avoided at all cost.
> 
> typedef struct my_struct_s {
>    /* Š */
> } *my_struct_t;
> 
> Or
> 
> typedef int * foobar;
> 
> Even when the typedef name tells you it is a pointer Œp_foobar¹ is not a
> great solution as the p_ could mean something else and should be avoided.
> When reading the code you would have to find the typedef or macro to
> determine its type is just too difficult to work with IMO.
> 
> The above is just asking for trouble and confusion IMO as the developer
> needs to make sure the typedef or macro is not hiding a pointer. We should
> avoid at all cost hiding a pointer or [] in a typedef or macro.
> 

This is exactly what the document is stating - except doing so via just not
using typedefs or macros for types. :-)

> Most of the functions in DPDK are like this:
> 
> int
> rte_some_function(struct my_struct * foo, struct my_struct2 * bar, struct
> my_struct3 * foorbar)
> 
> I believe the line is better in this fashion, less typing :-), but really
> it helps in reading, plus the line may not have to be broken across two or
> more lines.
> 
> int
> rte_some_function(my_struct_t * foo, my_struct2_t * bar, my_struct3_t *
> foobar)
> 
> (Not the best example, but you get the picture I hope)
> 
> BTW, I find having a space between the pointer and variable to be more
> readable. The pointer is an attribute of the variable nothing more and not
> having a space does not add to readability. Now having more then one space
> between is way too much and can be very hard to read IMO.
> 
> int *			foo;
> char **			bar;
> 
> Should be:
> 
> int	* foo;
> char	* bar;
> 
> Removing the space just makes the pointer symbol and variable name harder
> to read IMO.
> 
> int	*foo;
> char	*bar;
> 
> Some limit use of white space is good.
> 
> >
> >
> >C Function Definition, Declaration and Use
> >
> >Prototypes
> >
> >It is recommended, but not required that all functions are prototyped
> >somewhere. 
> >
> >Any function prototypes for private functions (that is, functions not
> >used elsewhere) go at the top of the first source module. Functions
> >local to one source module should be declared static.
> >
> >Functions used from other parts of code (external API) must be prototyped
> >in the relevant include file.
> >Function prototypes should be listed in a logical order, preferably
> >alphabetical unless there is a compelling reason to use a different
> >ordering. 
> >
> >Functions that are used locally in more than one module go into a
> >separate header file, for example, "extern.h".
> >
> >Do not use the ``__P`` macro.
> >
> >Functions that are part of an external API should be documented using
> >Doxygen-like comments above declarations. See the Doxgen documentation
> >topic for details.
> >
> >Associate names with parameter types, for example:
> > void function(int fd);
> >
> >
> >Short function prototypes should be contained on a single line. Longer
> >prototypes, e.g. those with many parameters,
> >can be split across multiple lines. Multi-line prototypes should use
> >space-indentation to enable function parameters to line up:
> > static char *function1(int _arg, const char *_arg2,
> >                      struct foo *_arg3,
> >                      struct bar *_arg4,
> >                      struct baz *_arg5);
> > static void usage(void);
> >
> >
> >Definitions
> >-----------
> >
> >The function type should be on a line by itself preceding the function.
> >The opening brace of the function body should be on a line by itself.
> > static char *
> > function(int a1, int a2, float fl, int a4)
> > {
> >
> >
> >Do not declare functions inside other functions. ANSI C states that such
> >declarations have file scope regardless of the nesting of the
> >declaration. 
> >Hiding file declarations in what appears to be a local scope is
> >undesirable and will elicit complaints from a good compiler.
> >
> >Old-style (K&R) function declaration should not be used, use ANSI
> >function declarations instead as shown below. Long argument lists
> >should be wrapped as described above in the function prototypes section.
> > /*
> >  * All major routines should have a comment briefly describing what
> >  * they do. The comment before the "main" routine should describe
> >  * what the program does.
> >  */
> > int
> > main(int argc, char *argv[])
> > {
> >         char *ep;
> >         long num;
> >         int ch;
> >
> >
> >C Command Line Parsing
> >----------------------
> >
> >For consistency, getopt(3) should be used to parse options. Options
> >should be sorted in the getopt(3) call and the switch statement,
> >unless parts of the switch cascade. Elements in a switch statement that
> >cascade should have a FALLTHROUGH comment.
> >Numerical arguments should be checked for accuracy. Code that cannot be
> >reached should have a NOTREACHED comment.
> > while ((ch = getopt(argc, argv, "abNn:")) != -1)
> >         switch (ch) {         /* Indent the switch. */
> >         case 'a':             /* Don't indent the case. */
> >                 aflag = 1;    /* Indent case body one tab. */
> >                 /* FALLTHROUGH */
> >         case 'b':
> >                 bflag = 1;
> >                 break;
> >         case 'N':
> >                 Nflag = 1;
> >                 break;
> >         case 'n':
> >                 num = strtol(optarg, &ep, 10);
> >                 if (num <= 0 || *ep != '\0') {
> >                         warnx("illegal number, -n argument -- %s",
> >                               optarg);
> >                         usage();
> >                 }
> >                 break;
> >         case '?':
> >         default:
> >                 usage();
> >                 /* NOTREACHED */
> >         }
> > argc -= optind;
> > argv += optind;
> >
> >
> >
> >
> >
> >C Indentation
> >-------------
> >
> >Control Statements and Loops
> >
> >Include a space after keywords (if, while, for, return, switch). Do not
> >use braces (``{`` and ``}``) for control statements with zero or just a
> >single statement, unless that statement is more than a single line in
> >which case the braces are permitted. Forever loops are done with for
> >statements, not while statements.
> > for (p = buf; *p != '\0'; ++p)
> >         ;       /* nothing */
> > for (;;)
> >         stmt;
> > for (;;) {
> >         z = a + really + long + statement + that + needs +
> >                 two + lines + gets + indented + on + the +
> >                 second + and + subsequent + lines;
> > }
> > for (;;) {
> >         if (cond)
> >                 stmt;
> > }
> > if (val != NULL)
> >         val = realloc(val, newsize);
> >
> >
> >Parts of a for loop may be left empty. It is recommended that you do not
> >put declarations inside blocks unless the routine is unusually
> >complicated. 
> > for (; cnt < 15; cnt++) {
> >         stmt1;
> >         stmt2;
> > }
> >
> >
> >Indentation is a hard tab, that is, a tab character, not a sequence of
> >spaces. 
> >NOTE General rule in DPDK, use tabs for indentation, spaces for
> >alignment. 
> >If you have to wrap a long statement, put the operator at the end of the
> >line, and indent again. For control statements (if, while, etc.),
> >it is recommended that the next line be indented by two tabs, rather than
> >one, to prevent confusion as to whether the second line of the
> >control statement forms part of the statement body or not. For
> >non-control statements, this issue does not apply, so they can be
> >indented 
> >by a single tab. However, a two-tab indent is recommended in this case
> >also to keep consistency across all statement types.
> > while (really_long_variable_name_1 == really_long_variable_name_2 &&
> >     var3 == var4){
> >     x = y + z;      /* control stmt body lines up with second line of */
> >     a = b + c;      /* control statement itself if single indent used */
> > }
> > 
> > if (really_long_variable_name_1 == really_long_variable_name_2 &&
> >         var3 == var4){  /* two tabs used */
> >     x = y + z;          /* statement body no longer lines up */
> >     a = b + c;
> > }
> > 
> > z = a + really + long + statement + that + needs +
> >         two + lines + gets + indented + on + the +
> >         second + and + subsequent + lines;
> >
> >
> >Do not add whitespace at the end of a line.
> >
> >Closing and opening braces go on the same line as the else keyword.
> >Braces that are not necessary should be left out.
> > if (test)
> >         stmt;
> > else if (bar) {
> >         stmt;
> >         stmt;
> > } else
> >         stmt;
> >
> >
> >Function Calls
> >--------------
> >
> >Do not use spaces after function names. Commas should have a space after
> >them. No spaces after ``(`` or ``[`` or preceding the ``]`` or ``)``
> >characters. 
> > error = function(a1, a2);
> > if (error != 0)
> >         exit(error);
> 
> I find the following OK as it tends to be more readable:
> if ( error != 0 )
> 	exit( error );
> 
> Again some space can be more readable. We have a space key use it wisely
> :-)

While I would be ok with making a change like this if everyone likes it, I'm not
sure it helps that much. It's downsides are:
* we break compatibility with the majority of our existing codebase
* we would have to fork checkpatch to stop throwing errors at something like this.

The downsides seem to outweigh the positives for me.

> >
> >
> >Operators
> >---------
> >
> >Unary operators do not require spaces, binary operators do. Do not use
> >parentheses unless they are required for precedence or unless the
> >statement is confusing without them. Remember that other people may be
> >more easily confused than you.
> >
> >Exit
> >
> >Exits should be 0 on success, or 1 on failure.
> >         exit(0);        /*
> >                          * Avoid obvious comments such as
> >                          * "Exit 0 on success."
> >                          */
> > }
> >
> >
> >Local Variables
> >---------------
> >
> >When declaring variables in functions, declare them sorted by size, then
> >in alphabetical order. Multiple variables per line are OK.
> >If a line overflows reuse the type keyword.
> >
> >Be careful to not obfuscate the code by initializing variables in the
> >declarations, only the last variable on a line should be initialized.
> >If multiple variables are to be initialised when defined, put one per
> >line. Do not use function calls in initializers.
> > int i = 0, j = 0, k = 0;  /* bad, too many initializer */
> > 
> > char a = 0;        /* OK, one variable per line with initializer */
> > char b = 0;
> > 
> > float x, y = 0.0;  /* OK, only last variable has initializer */
> >
> >
> >Casts and sizeof
> >
> >Casts and sizeof statements are not followed by a space. Always write
> >sizeof statements with parenthesis.
> >The redundant parenthesis rules do not apply to sizeof(var) instances.
> >
> >C Style and Conventions
> >
> >NULL Pointers
> >
> >NULL is the preferred null pointer constant. Use NULL instead of ``(type
> >*)0`` or ``(type *)NULL`` in contexts where the compiler knows the type,
> >for example, in assignments. Use ``(type *)NULL`` in other contexts, in
> >particular for all function args.
> >(Casting is essential for variadic args and is necessary for other args
> >if the function prototype might not be in scope.) Test pointers against
> >NULL, for example, use::
> > (p = f()) == NULL
> >
> >
> >not:: 
> > !(p = f())
> >
> >
> >Do not use ! for tests unless it is a boolean, for example, use::
> > if (*p == '\0')
> >
> >
> >not:: 
> > if (!*p)
> >
> >
> >Return Value
> >------------
> >
> >If possible, functions should return 0 on success and a negative value on
> >error. The negative value should be ``-errno`` if relevant, for example,
> >``-EINVAL``. 
> >
> >Routines returning ``void *`` should not have their return values cast to
> >any pointer type. 
> >(Typecasting can prevent the compiler from warning about missing
> >prototypes as any implicit definition of a function returns int - which,
> >unlike "void *" needs a typecast to assign to a pointer variable.)
> >NOTE The above rule applies to malloc, as well as to DPDK functions.
> >Values in return statements should be enclosed in parentheses.
> >
> >Logging and Errors
> >------------------
> >
> >In the DPDK environment, use the logging interface provided::
> > #define RTE_LOGTYPE_TESTAPP1 RTE_LOGTYPE_USER1
> > #define RTE_LOGTYPE_TESTAPP2 RTE_LOGTYPE_USER2
> > 
> > /* enable these logs type */
> > rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1);
> > rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1);
> > 
> > /* log in debug level */
> > rte_set_log_level(RTE_LOG_DEBUG);
> > RTE_LOG(DEBUG, TESTAPP1, "this is is a debug level message\n");
> > RTE_LOG(INFO, TESTAPP1, "this is is a info level message\n");
> > RTE_LOG(WARNING, TESTAPP1, "this is is a warning level message\n");
> > 
> > /* log in info level */
> > rte_set_log_level(RTE_LOG_INFO);
> > RTE_LOG(DEBUG, TESTAPP2, "debug level message (not displayed)\n");
> >
> >
> >In a userland program that is not a DPDK application, use err(3) or
> >warn(3). Do not create your own variant.
> >         if ((four = malloc(sizeof(struct foo))) == NULL)
> >                 err(1, (char *)NULL);
> >         if ((six = (int *)overflow()) == NULL)
> >                 errx(1, "number overflowed");
> >         return (eight);
> > }
> >
> >
> >Variable Arguments List
> >-----------------------
> >
> >Variable numbers of arguments should look like this:
> > #include <stdarg.h>
> > 
> > void
> > vaf(const char *fmt, ...)
> > {
> >         va_list ap;
> > 
> >         va_start(ap, fmt);
> >         STUFF;
> >         va_end(ap);
> >         /* No return needed for void functions. */
> > }
> > 
> > static void
> > usage()
> > {
> >         /* Insert an empty line if the function has no local variables.
> >*/
> >
> >
> >Printf
> >------
> >
> >Use printf(3), not fputs(3), puts(3), putchar(3) or whatever. It is
> >faster and usually cleaner, and helps to avoid unnecessary bugs. However,
> >be aware of format string bugs::
> > int
> > main(int argc, char **argv)
> > {
> >         if(argc != 2)
> >             exit(1);
> >         printf(argv[1]); /* bad ! */
> >         printf("%s", argv[1]); /* correct */
> >
> >
> >Usage
> >-----
> >
> >Usage statements should look like the manual pages SYNOPSIS. The usage
> >statement should be structured in the following order:
> >1. Options without operands come first, in alphabetical order, inside a
> >single set of brackets (``[`` and ``]``).
> >2. Options with operands come next, also in alphabetical order, with each
> >option and its argument inside its own pair of brackets.
> >3. Required arguments (if any) are next, listed in the order they should
> >be specified on the command line.
> >4. Finally, any optional arguments, listed in the order they should be
> >specified, and all inside brackets.
> >
> >A bar (`|') separates ``either-or`` options/arguments, and multiple
> >options/arguments, which are specified together, are placed in a single
> >set of brackets. 
> > "usage: f [-aDde] [-b b_arg] [-m m_arg] req1 req2 [opt1 [opt2]]\n"
> > "usage: f [-a | -b] [-c [-dEe] [-n number]]\n"
> > 
> > (void)fprintf(stderr, "usage: f [-ab]\n");
> >         exit(1);
> > }
> >
> >
> >Note that the manual page options description should list the options in
> >pure alphabetical order. That is, without regard to
> >whether an option takes arguments or not. The alphabetical ordering
> >should take into account the case ordering shown above.
> >
> >Branch Prediction
> >-----------------
> >
> >When a test is done in a critical zone (called often or in a data path)
> >use the ``likely()`` and ``unlikely()`` macros. They are expanded
> >as a compiler builtin and allow the developer to indicate if the branch
> >is likely to be taken or not. Example:
> > #include <rte_branch_prediction.h>
> > if (likely(x > 1))
> >   do_stuff();
> >
> >
> >Static Variables and Functions
> >------------------------------
> >
> >All functions and variables that are local to a file must be declared as
> >``static`` because it can often help the compiler to do
> >some optimizations (such as, inlining the code).
> >
> >Functions that must be inlined have to be declared as ``static inline``
> >and can be defined in a .c or a .h file.
> >
> >Const Attribute
> >---------------
> >
> >Particular care must be taken with the use of the ``const`` attribute. It
> >should be used as often as possible when a variable is read-only.
> >
> >ASM Coding Rules
> >----------------
> >
> >Assembly Syntax
> >
> >NASM is used for assembly, with the syntax, therefore guidelines given
> >here are appropriate to this target.
> >[GNU as is intended to support both syntax variants, but that is not
> >documented here]. The following general guidelines are valid in any case.
> > globals, extern and macros are to be defined at the top of the file
> > labels should stay explicit, and are left aligned
> > code is indented with a tabulation, no spaces
> > instruction and operands should be separated by a tab too
> > code should be separated in blocks
> > blocks, when possible, should start with a comment explanation
> >
> >Sample code: 
> > ; comment header
> > 
> > ; export this symbol
> > [GLOBAL entry]
> > 
> > ; external variables and functions
> > [EXTERN variable]
> > 
> > ; 16 bits code
> > [BITS 16]
> > 
> > ; macros like
> > BIOS_START  EQU     0x7C00
> > 
> > entry:
> > 
> >       ; Clear interrupt flag
> >       cli
> > 
> >       ; Set segment registers to 0
> >       xor     bx, bx
> >       mov     es, bx
> >       mov     fs, bx
> >       mov     gs, bx
> >       mov     ds, bx
> >       mov     ss, bx
> >       mov     sp, 0x7C00
> >       sti
> > 
> >;; [...] snip [...]
> >
> >
> >Use of C-style macros is allowed. When compiling ASM code, a file is
> >parsed by the C preprocessor. It is then allowed to share some constants
> >between C and assembly code, in a ``.h`` file.
> >
> >Inline ASM in C code
> >
> >The ``asm`` and ``volatile`` keywords do not have underscores. The AT&T
> >syntax should be used. Input and output operands should be named to avoid
> >confusion, 
> >as shown in the following example::
> > asm volatile("outb %[val], %[port]"
> > : :
> > [port] "dN" (port),
> > [val] "a" (val));
> >
> >
> >Environment or Architecture-specific Sources
> >
> >In DPDK and DPDK applications, some code is specific to an architecture
> >(i686, x86_64) or to an executive environment (bare-metal or linuxapp)
> >and so on. 
> >
> >There are several ways to handle specific code:
> > Use a ``#ifdef`` with the CONFIG option in the C code. This can be done
> >when the differences are small and they can be embedded in the same C
> >file:: 
> >   #ifdef RTE_ARCH_I686
> >   toto();
> >   #else
> >   titi();
> >   #endif
> >
> >Use the CONFIG option in the Makefile. This is done when the differences
> >are more significant. In this case, the code is split into
> >two separate files that are architecture or environment specific.
> >
> >The same logic applies to header files.
> >
> >By convention, a file is common if it is not located in a directory
> >indicating that it is specific. For instance, a file located in a
> >subdir of "x86_64" directory is specific to this architecture. A file
> >located in a subdir of "linuxapp" is specific to this execution
> >environment. 
> >NOTE As in the linux kernel, the "CONFIG_" prefix is not used in C code.
> >This is only needed in Makefiles or shell scripts.
> >Per Architecture Sources
> >
> >The following config options can be used:
> > CONFIG_RTE_ARCH is a string that contains the name of the architecture.
> > CONFIG_RTE_ARCH_I686, CONFIG_RTE_ARCH_X86_64 or
> >CONFIG_RTE_ARCH_X86_64_32 are defined only if we are building for those
> >architectures. 
> >
> >Per Execution Environment Sources
> >
> >The following config options can be used:
> > CONFIG_RTE_EXEC_ENV is a string that contains the name of the executive
> >environment. 
> > CONFIG_RTE_EXEC_ENV_BAREMETAL or CONFIG_RTE_EXEC_ENV_LINUXAPP are
> >defined only if we are building for this execution environment.
> >
> >Per Driver Sources
> > RTE_LIBRTE_IGB_PMD or RTE_LIBRTE_IXGBE_PMD are defined only if we are
> >using this driver.
> >
> >Doxygen Documentation
> >---------------------
> >
> >The API documentation is automatically generated in the DPDK framework. 
> >That is why all files that are part of the public 
> >API must be documented using Doxygen syntax. 
> >
> >The public API comprises functions of DPDK that can be used by an 
> >external application that will use the SDK. Only the Doxygen 
> >syntax described in the coding rules (this document) should be used in 
> >the code. All the Doxygen features are described in the Doxygen manual 
> >online. 
> >
> >Documenting a Function
> >
> >All public functions must be documented. The documentation is placed in 
> >the header file, above the declaration of the function. 
> >The definition of the function may be documented, but using standard 
> >comments (not in doxygen format). 
> >Private functions can be documented using Doxygen. The following is an 
> >example of function documentation: 
> > /**
> >  * Summary here; one sentence on one line (should not exceed 80 chars).
> >  *
> >  * A more detailed description goes here.
> >  *
> >  * A blank line forms a paragraph. There should be no trailing 
> >white-space
> >  * anywhere.
> >  *
> >  * @param first
> >  *   "@param" is a Doxygen directive to describe a function parameter. 
> >Like
> >  *   some other directives, it takes a term/summary on the same line and 
> >a
> >  *   description (this text) indented by 2 spaces on the next line. All
> >  *   descriptive text should wrap at 80 chars, without going over.
> >  *   Newlines are NOT supported within directives; if a newline would be
> >  *   before this text, it would be appended to the general description 
> >above.
> >  * @param second
> >  *   There should be no newline between multiple directives (of the same
> >  *   type).
> >  *
> >  * @return
> >  *   "@return" is a different Doxygen directive to describe the return 
> >value
> >  *   of a function, if there is any.
> >  */
> > int rte_foo(int first, int second)
> >
> >
> >Documenting Files
> >
> >Each public file may start with a comment describing what the file does. 
> >For example: 
> > /**
> >  * @file
> >  * This file describes the coding rules of RTE.
> >  *
> >  * It contains the coding rules of C code, ASM code, reStructured
> >  * Text documentation, and of course how to use doxygen to document
> >  * public API.
> >  */
> >
> >
> >Documenting Constants and Variables
> > /**
> >  * The definition of a funny TRUE.
> >  */
> > #define TRUE 0
> >
> >
> >
> >
> > #define TRUE 1 /**< another way to document a macro */
> >
> > /**
> >  * Frequency of the HPET counter in Hz
> >  *
> >  * @see rte_eal_hpet_init()
> >  */
> > extern uint64_t eal_hpet_resolution_hz;
> >
> >
> >Documenting Structures
> >
> >Public structures should also be documented. The ``/**<`` sequence can be 
> >used to documented the fields of the structure, as shown in the following 
> >example: 
> > /**
> >  * Structure describing a memzone, which is a contiguous portions of
> >  * physical memory identified by a name.
> >  */
> > struct rte_memzone {
> > 
> > #define MEMZONE_NAMESIZE 32
> >   char name[MEMZONE_NAMESIZE]; /**< name of the memory zone */
> > 
> >   phys_addr_t phys_addr;       /**< start physical address */
> >   void *addr;                  /**< start virtual address */
> >   uint64_t len;                /**< len of the memzone */
> > 
> >   int socket_id;               /**< NUMA socket id */
> > };
> >
> >
> >Using Lists
> >
> >Using the minus character, it is possible to generate a bullet list. The 
> >minus signs must be column-aligned. If the minus sign is followed by a 
> >hash, 
> >then it generates an enumerated list. Refer to the official Doxygen 
> >documentation for more information. 
> > /**
> >  *  A list of events:
> >  *    - mouse events
> >  *         -# mouse move event
> >  *         -# mouse click event\n
> >  *            More info about the click event.
> >  *         -# mouse double click event
> >  *    - keyboard events
> >  *         -# key down event
> >  *         -# key up event
> >  *
> >  *  More text here.
> >  */
> >
> >
> >See Also Sections
> >
> >The @see keyword can be used to highlight a link to an existing function, 
> >file, or URL. This directive should be placed on one line, without 
> >anything else, at the bottom of the documentation header. 
> > /**
> >  * (documentation of function, file, ...)
> >  *
> >  * @see rte_foo()
> >  * @see eal_memzone.c
> >  */
> > 
>