From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bruce Richardson Subject: Re: tools brainstorming Date: Tue, 14 Apr 2015 15:50:48 +0100 Message-ID: <20150414145048.GB3296@bricha3-MOBL3> References: <3571725.20GtF5MAnU@xps13> <0C5AFCA4B3408848ADF2A3073F7D8CC86D58F9C2@IRSMSX109.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: "dev-VfR2kkLFssw@public.gmane.org" To: "Wiles, Keith" Return-path: Content-Disposition: inline In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" On Wed, Apr 08, 2015 at 02:16:55PM +0000, Wiles, Keith wrote: >=20 >=20 > On 4/8/15, 5:43 AM, "Butler, Siobhan A" wr= ote: >=20 > >Hi all, > >To add to the tools brainstorming - I propose we use the following Cod= ing > >Standards as the basis of guidelines on coding style going forward. > >The style outlined below is in alignment with the current convention u= sed > >for the majority of the project. > >Any thoughts/suggestions or feedback welcome. > >Thanks > >Siobhan :) > > > > > > > > > >Coding Style > >~~~~~~~~~~ > > > >Description > >----------- > > > >This document specifies the preferred style for source files in the DP= DK > >source tree.=20 > >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.=20 > > > >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:=20 > >The code style should be consistent within each individual file, and > >within each file in a given directory or module - in the case of creat= ing > >new files=20 > >The primary reason for coding standards is to increase code readabilit= y > >and comprehensibility, therefore always use whatever option will make = the > >code easiest to read. > > > >The following more specific recommendations apply to all sections, bot= h > >for C and assembly code: > >Line length is recommended to be not more than 80 characters, includin= g > >comments. [Tab stop size should be assumed to be at least 4-characters > >wide]=20 > >Indentation should be to no more than 3 levels deep. > >NOTE The above are recommendations, and not hard limits. However, it i= s > >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 AP= I, > >a doxygen-like format must be used: refer to Doxygen Documentation. > > /* > > * VERY important single-line comments look like this. > > */ > >=20 > > /* Most single-line comments look like this. */ > >=20 > > /* > > * Multi-line comments look like this. Make them real sentences. Fil= l > > * them so they look like real paragraphs. > > */ >=20 > Did you mean to have the =C5=92*=C2=B9 aligned, if so good, if not then= it does not > make sense to not align them. The indent of one space here does not hel= p > convey any information IMO. > > > >License Header > >-------------- > > > >Each file should begin with a special comment tag which will contain t= he > >appropriate copyright and license for the file (Generally BSD License)= . > >After any copyright header, a blank line should be left before any oth= er > >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:=20 > > #include > > #include > >=20 > > #include > >=20 > > #include > > #include > >=20 > > #include "application.h" > > > > > >Global pathnames are defined in . Pathnames local to the prog= ram > >go in "pathnames.h" in the local directory. > > #include > > > > > >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_ > >=20 > > /* Code */ > >=20 > > #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 functio= n, > >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 i= f > >statements.=20 > >Any final statement-terminating semicolon should be supplied by the ma= cro > >invocation rather than the macro, to make parsing easier for > >pretty-printers and editors. > > #define MACRO(x, y) do { \ > > variable =3D (x) + (y); \ > > (y) +=3D 2; \ > > }while (0) >=20 > I have seen this =C5=92while( /*CONSTCOND*/0 )=C2=B9 (I think that is = the comment) > plus I have see this =C5=92while((0))=C2=B9 as gcc complained about a c= onstant 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), si= nce this is the generally recommended way to do define code blocks in macros. > > > >NOTE Wherever possible, enums and typedefs should be preferred to macr= os, > >since they provide additional degrees > >of type-safety and can allow compilers to emit extra warnings about > >unsafe code.=20 > > > >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 compi= led > >for the purposes of lint(1), even though the uncompiled > >region may be small. The comment should be separated from the #endif o= r > >#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 #i= f > >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 > > #endif > >=20 > > #ifdef COMPAT_43 > > /* A large region here, or other conditional code. */ > > #else /* !COMPAT_43 */ > > /* Or here. */ > > #endif /* COMPAT_43 */ > >=20 > > #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.=20 > >C Types > > > >Integers > > > >For fixed/minimum-size integer values, the project uses the form uintX= X_t > >(from stdint.h) instead of older BSD-style integer identifiers of the > >form u_intXX_t.=20 > > > >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; > > /* ... */ > > } >=20 > I like to line up the =C5=92:=C2=B9 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 adjac= ent > >tokens, except for tokens that are identifiers related to types. >=20 > Much more readable to have the space between pointer and token IMO. >=20 This is not the way things are done right now in our code, and I see no r= eason to change it. [Does checkpatch complain if you insert spaces?] > >=20 > >(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. >=20 > 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 */ >=20 > >=20 > > > >Structure Declarations > > > >When declaring variables in structures, declare them sorted by use, th= en > >by size (largest to smallest), and then in alphabetical order. > >Alignment constraints may override the previous rules. The first categ= ory > >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. >=20 > Sorting variables in structures and other places just seem to much > trouble. Sometimes you want to group variables together as they are use= d > together in the code below. Hence the "sorted by use" - where appropriate. >=20 > Ordering structure members is not going work as alignment and grouping = is > more important. Alignment is important as we all know holes in structur= es > 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 app= ly, i.e. in most cases, we hope, we group by size from largest to smallest, t= hereby avoiding wasting space with packing i.e. holes. Overall, I think the abov= e guideline is pretty reasonable. > Sorting local variables is also not reasonable, for grouping of variabl= es > and being able to maintain the order. >=20 > >=20 > > 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 membe= rs > >*/ > > }; > > > > > >Major structures should be declared at the top of the file in which th= ey > >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 fil= e. > > > >Queues > > > >Use queue(3) macros rather than rolling your own lists, whenever > >possible. Thus, the previous example would be better written: > > #include > >=20 > > 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 membe= rs > >*/ > > }; > > LIST_HEAD(, foo) foohead; /* Head of global foo list. */ > > > > > >DPDK also provides an optimized way to store elements in lockless ring= s. > >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 { > > /* ... */ > > }; > >=20 > > struct my_struct_type my_var; > > > > > >rather than:=20 > > typedef struct my_struct_type { > > /* ... */ > > } my_struct_type; > >=20 > > 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. I= n > >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. >=20 > 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 type= s. > 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 defi= ne > the following: >=20 > typedef struct my_struct_s { > /* =C5=A0 */ > } my_struct_t; >=20 > Do not do the following: >=20 > struct my_struct { > /* =C5=A0 */ > }; > typedef struct my_struct my_struct_t; >=20 > 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. >=20 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 usin= g them, as I believe the objections to their use given in the proposed document a= re still valid. I also think having to resort to prefixes is ugly, and if we did start us= ing typedefs with _t prefixes in place of structures, we end up with very dif= ferent 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), >=20 > Namespace pollution, I do not fully understand the concern here. You should not have to include a whole header file just to get a particul= ar=20 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; > >=20 > >or there must be a back-door mechanism for obtaining the typedef. >=20 > 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 th= e > use of the =C5=92struct my_struct_s=C2=B9 instead of the typedef is OK,= but we > should try to fix the ordering problem. Today we have =C5=92struct my_s= truct=C2=B9 > to forward declare structures when the header is not included, which se= ems > to be a back-door solution, correct? >=20 Or we can just continue to not use typedefs, in which case there is no is= sue :-) >=20 > >=20 > >NOTE #defines used instead of typedefs also are problematic (since the= y > >do not propagate the pointer type correctly due to direct text > >replacement).=20 > >For example, ``#define pint int *`` does not work as expected, while > >``typedef int *pint`` does work. As stated when discussing macros, > >typedefs=20 > >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.=20 > > /* 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. */ >=20 > This one does not make sense the =C5=92BAR=C2=B9 to me looks like a MAC= RO not a > typedef, this is why using the =C5=92bar_t' solution makes the most sen= se. >=20 > 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. >=20 > typedef struct my_struct_s { > /* =C5=A0 */ > } *my_struct_t; >=20 > Or >=20 > typedef int * foobar; >=20 > Even when the typedef name tells you it is a pointer =C5=92p_foobar=C2=B9= is not a > great solution as the p_ could mean something else and should be avoide= d. > 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. >=20 > 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 sho= uld > avoid at all cost hiding a pointer or [] in a typedef or macro. >=20 This is exactly what the document is stating - except doing so via just n= ot using typedefs or macros for types. :-) > Most of the functions in DPDK are like this: >=20 > int > rte_some_function(struct my_struct * foo, struct my_struct2 * bar, stru= ct > my_struct3 * foorbar) >=20 > I believe the line is better in this fashion, less typing :-), but real= ly > it helps in reading, plus the line may not have to be broken across two= or > more lines. >=20 > int > rte_some_function(my_struct_t * foo, my_struct2_t * bar, my_struct3_t * > foobar) >=20 > (Not the best example, but you get the picture I hope) >=20 > 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 sp= ace > between is way too much and can be very hard to read IMO. >=20 > int * foo; > char ** bar; >=20 > Should be: >=20 > int * foo; > char * bar; >=20 > Removing the space just makes the pointer symbol and variable name hard= er > to read IMO. >=20 > int *foo; > char *bar; >=20 > Some limit use of white space is good. >=20 > > > > > >C Function Definition, Declaration and Use > > > >Prototypes > > > >It is recommended, but not required that all functions are prototyped > >somewhere.=20 > > > >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 prototy= ped > >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.=20 > > > >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 su= ch > >declarations have file scope regardless of the nesting of the > >declaration.=20 > >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 sectio= n. > > /* > > * 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 tha= t > >cascade should have a FALLTHROUGH comment. > >Numerical arguments should be checked for accuracy. Code that cannot b= e > >reached should have a NOTREACHED comment. > > while ((ch =3D getopt(argc, argv, "abNn:")) !=3D -1) > > switch (ch) { /* Indent the switch. */ > > case 'a': /* Don't indent the case. */ > > aflag =3D 1; /* Indent case body one tab. */ > > /* FALLTHROUGH */ > > case 'b': > > bflag =3D 1; > > break; > > case 'N': > > Nflag =3D 1; > > break; > > case 'n': > > num =3D strtol(optarg, &ep, 10); > > if (num <=3D 0 || *ep !=3D '\0') { > > warnx("illegal number, -n argument -- %s", > > optarg); > > usage(); > > } > > break; > > case '?': > > default: > > usage(); > > /* NOTREACHED */ > > } > > argc -=3D optind; > > argv +=3D optind; > > > > > > > > > > > >C Indentation > >------------- > > > >Control Statements and Loops > > > >Include a space after keywords (if, while, for, return, switch). Do no= t > >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 =3D buf; *p !=3D '\0'; ++p) > > ; /* nothing */ > > for (;;) > > stmt; > > for (;;) { > > z =3D a + really + long + statement + that + needs + > > two + lines + gets + indented + on + the + > > second + and + subsequent + lines; > > } > > for (;;) { > > if (cond) > > stmt; > > } > > if (val !=3D NULL) > > val =3D realloc(val, newsize); > > > > > >Parts of a for loop may be left empty. It is recommended that you do n= ot > >put declarations inside blocks unless the routine is unusually > >complicated.=20 > > for (; cnt < 15; cnt++) { > > stmt1; > > stmt2; > > } > > > > > >Indentation is a hard tab, that is, a tab character, not a sequence of > >spaces.=20 > >NOTE General rule in DPDK, use tabs for indentation, spaces for > >alignment.=20 > >If you have to wrap a long statement, put the operator at the end of t= he > >line, and indent again. For control statements (if, while, etc.), > >it is recommended that the next line be indented by two tabs, rather t= han > >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=20 > >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 =3D=3D really_long_variable_name_2= && > > var3 =3D=3D var4){ > > x =3D y + z; /* control stmt body lines up with second line = of */ > > a =3D b + c; /* control statement itself if single indent us= ed */ > > } > >=20 > > if (really_long_variable_name_1 =3D=3D really_long_variable_name_2 && > > var3 =3D=3D var4){ /* two tabs used */ > > x =3D y + z; /* statement body no longer lines up */ > > a =3D b + c; > > } > >=20 > > z =3D 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 aft= er > >them. No spaces after ``(`` or ``[`` or preceding the ``]`` or ``)`` > >characters.=20 > > error =3D function(a1, a2); > > if (error !=3D 0) > > exit(error); >=20 > I find the following OK as it tends to be more readable: > if ( error !=3D 0 ) > exit( error ); >=20 > Again some space can be more readable. We have a space key use it wisel= y > :-) 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 l= ike 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, th= en > >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 =3D 0, j =3D 0, k =3D 0; /* bad, too many initializer */ > >=20 > > char a =3D 0; /* OK, one variable per line with initializer */ > > char b =3D 0; > >=20 > > float x, y =3D 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 ``(ty= pe > >*)0`` or ``(type *)NULL`` in contexts where the compiler knows the typ= e, > >for example, in assignments. Use ``(type *)NULL`` in other contexts, i= n > >particular for all function args. > >(Casting is essential for variadic args and is necessary for other arg= s > >if the function prototype might not be in scope.) Test pointers agains= t > >NULL, for example, use:: > > (p =3D f()) =3D=3D NULL > > > > > >not::=20 > > !(p =3D f()) > > > > > >Do not use ! for tests unless it is a boolean, for example, use:: > > if (*p =3D=3D '\0') > > > > > >not::=20 > > 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 exampl= e, > >``-EINVAL``.=20 > > > >Routines returning ``void *`` should not have their return values cast= to > >any pointer type.=20 > >(Typecasting can prevent the compiler from warning about missing > >prototypes as any implicit definition of a function returns int - whic= h, > >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 > >=20 > > /* enable these logs type */ > > rte_set_log_type(RTE_LOGTYPE_TESTAPP1, 1); > > rte_set_log_type(RTE_LOGTYPE_TESTAPP2, 1); > >=20 > > /* 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"); > >=20 > > /* 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 =3D malloc(sizeof(struct foo))) =3D=3D NULL) > > err(1, (char *)NULL); > > if ((six =3D (int *)overflow()) =3D=3D NULL) > > errx(1, "number overflowed"); > > return (eight); > > } > > > > > >Variable Arguments List > >----------------------- > > > >Variable numbers of arguments should look like this: > > #include > >=20 > > void > > vaf(const char *fmt, ...) > > { > > va_list ap; > >=20 > > va_start(ap, fmt); > > STUFF; > > va_end(ap); > > /* No return needed for void functions. */ > > } > >=20 > > static void > > usage() > > { > > /* Insert an empty line if the function has no local variable= s. > >*/ > > > > > >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. Howev= er, > >be aware of format string bugs:: > > int > > main(int argc, char **argv) > > { > > if(argc !=3D 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 e= ach > >option and its argument inside its own pair of brackets. > >3. Required arguments (if any) are next, listed in the order they shou= ld > >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 singl= e > >set of brackets.=20 > > "usage: f [-aDde] [-b b_arg] [-m m_arg] req1 req2 [opt1 [opt2]]\n" > > "usage: f [-a | -b] [-c [-dEe] [-n number]]\n" > >=20 > > (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 branc= h > >is likely to be taken or not. Example: > > #include > > 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 ca= se. > > 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:=20 > > ; comment header > >=20 > > ; export this symbol > > [GLOBAL entry] > >=20 > > ; external variables and functions > > [EXTERN variable] > >=20 > > ; 16 bits code > > [BITS 16] > >=20 > > ; macros like > > BIOS_START EQU 0x7C00 > >=20 > > entry: > >=20 > > ; Clear interrupt flag > > cli > >=20 > > ; 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 > >=20 > >;; [...] 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 constan= ts > >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 av= oid > >confusion,=20 > >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 architectur= e > >(i686, x86_64) or to an executive environment (bare-metal or linuxapp) > >and so on.=20 > > > >There are several ways to handle specific code: > > Use a ``#ifdef`` with the CONFIG option in the C code. This can be do= ne > >when the differences are small and they can be embedded in the same C > >file::=20 > > #ifdef RTE_ARCH_I686 > > toto(); > > #else > > titi(); > > #endif > > > >Use the CONFIG option in the Makefile. This is done when the differenc= es > >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.=20 > >NOTE As in the linux kernel, the "CONFIG_" prefix is not used in C cod= e. > >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 architectur= e. > > CONFIG_RTE_ARCH_I686, CONFIG_RTE_ARCH_X86_64 or > >CONFIG_RTE_ARCH_X86_64_32 are defined only if we are building for thos= e > >architectures.=20 > > > >Per Execution Environment Sources > > > >The following config options can be used: > > CONFIG_RTE_EXEC_ENV is a string that contains the name of the executi= ve > >environment.=20 > > 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= .=20 > >That is why all files that are part of the public=20 > >API must be documented using Doxygen syntax.=20 > > > >The public API comprises functions of DPDK that can be used by an=20 > >external application that will use the SDK. Only the Doxygen=20 > >syntax described in the coding rules (this document) should be used in= =20 > >the code. All the Doxygen features are described in the Doxygen manual= =20 > >online.=20 > > > >Documenting a Function > > > >All public functions must be documented. The documentation is placed i= n=20 > >the header file, above the declaration of the function.=20 > >The definition of the function may be documented, but using standard=20 > >comments (not in doxygen format).=20 > >Private functions can be documented using Doxygen. The following is an= =20 > >example of function documentation:=20 > > /** > > * 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=20 > >white-space > > * anywhere. > > * > > * @param first > > * "@param" is a Doxygen directive to describe a function parameter= .=20 > >Like > > * some other directives, it takes a term/summary on the same line = and=20 > >a > > * description (this text) indented by 2 spaces on the next line. A= ll > > * 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 descriptio= n=20 > >above. > > * @param second > > * There should be no newline between multiple directives (of the s= ame > > * type). > > * > > * @return > > * "@return" is a different Doxygen directive to describe the retur= n=20 > >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 doe= s.=20 > >For example:=20 > > /** > > * @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=20 > >used to documented the fields of the structure, as shown in the follow= ing=20 > >example:=20 > > /** > > * Structure describing a memzone, which is a contiguous portions of > > * physical memory identified by a name. > > */ > > struct rte_memzone { > >=20 > > #define MEMZONE_NAMESIZE 32 > > char name[MEMZONE_NAMESIZE]; /**< name of the memory zone */ > >=20 > > phys_addr_t phys_addr; /**< start physical address */ > > void *addr; /**< start virtual address */ > > uint64_t len; /**< len of the memzone */ > >=20 > > int socket_id; /**< NUMA socket id */ > > }; > > > > > >Using Lists > > > >Using the minus character, it is possible to generate a bullet list. T= he=20 > >minus signs must be column-aligned. If the minus sign is followed by a= =20 > >hash,=20 > >then it generates an enumerated list. Refer to the official Doxygen=20 > >documentation for more information.=20 > > /** > > * 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 functi= on,=20 > >file, or URL. This directive should be placed on one line, without=20 > >anything else, at the bottom of the documentation header.=20 > > /** > > * (documentation of function, file, ...) > > * > > * @see rte_foo() > > * @see eal_memzone.c > > */ > >=20 >=20