> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Wednesday, April 8, 2015 7:16 PM
> To: Butler, Siobhan A
> Cc: Thomas Monjalon; dev at dpdk.org
> Subject: Re: [dpdk-dev] tools brainstorming
>
> Thanks for doing this, it is a great start.
> I admit strong bias towards Linux kernel style.
>
> Could you use one of the standard markup styles so that it could get put in
> documentation?
Sure Stephen - will tidy it all up when we get a consensus :)
Thanks for the feedback
Siobhan
>
>
> > License Header
> > --------------
>
> I prefer the file just say that it is BSD or GPL and refer to license files
> in the
> package. That way if something has to change it doesn't need a massive
> license sweep
>
>
> >
> > 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)
> ^ bad whitespace
>
> it is important that all examples in documentation are perfect.
>
>
> > 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.
>
> I find prototypes for private functions to be redundant and error prone.
> The do nothing. Better to just put private functions in the correct order.
>
>
> You also need to raise the issue that all global names need to be prefaced by
> a unique string.
> I see places in drivers where global names leak out causing possible later
> name collision.
>
> > 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)
> > {
>
> Not a big fan of that style. Prefer it on same line.
>
>
> >
> > Indentation is a hard tab, that is, a tab character, not a sequence of
> > spaces.
>
> Also no spaces before tabs.
>
> > 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 */
>
> No. Should line up with really_long_variable_name_1
>
> > 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);
> >
> >
> > 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."
> > */
>
> +11
>
>
> >
> > 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.
>
> Sorry, this is a stupid BSDism
> return (-EINVAL);
> is ugly
>
> >
> > 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);
> ^ NO BSD style return ()
>
> >
> > 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();
>
> You need to stress that likely() and unlikely() should be used sparingly.
> Many times the compiler knows better.
>
> >
> >
> > 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.
>
>
> A couple more things:
> no UPPER or CamelCase variable names or functions
>
> The other thing (and Intel is the worst offender) is to avoid excessive
> inlining.
> For example, I am pretty sure that all rte_ring stuff really should not be
> inline,
> the compiler can do a good job (especially with LTO) even without inlining.
