From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f180.google.com (mail-wi0-f180.google.com [209.85.212.180]) by dpdk.org (Postfix) with ESMTP id EA8DA9A8A for ; Thu, 16 Apr 2015 12:50:17 +0200 (CEST) Received: by wizk4 with SMTP id k4so189491042wiz.1 for ; Thu, 16 Apr 2015 03:50:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=eAIv/81jRaINn7B4SkZXNLXDLpWXZNcwXWYYW6z+nVc=; b=GKFgInpUNRd2DZGVvAHv5PATS161oG9AGK2HcZjS7+0fxnfXQPpDbQ5tVKke/EaCJb XFmWgPHt6jTg8rKMJflV88kRzu0MnS9gChxysphFqoXcu3qARIGaGCa+wFPYCijARTBS S7gcFaMg2UEaDbjflj90zRGzkiUCdRBEvJXEs565k9aerV8xqSsSKDYrlTU1DINPyzbc eucso4VgEYumHPuDM2uUIaYYzbIrxOCS/Tp9XI8PlWwXyPGzMjbKrmyVAp1zflZRaFQj UqT1H3qdjZeb2bIrR7tF7zB2y+lfn7kkYyASW8zkYLyEmX20bjNhanl7jtSaxJbhEPsR z5DA== X-Gm-Message-State: ALoCoQnp9iEitBOCGKKwSJqtZrlNmRGORF99o9L6xIiuWKqhVWwRSKgMe38A3hXl0AYvhaRgKIAW X-Received: by 10.194.121.38 with SMTP id lh6mr37569329wjb.2.1429181416843; Thu, 16 Apr 2015 03:50:16 -0700 (PDT) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id js3sm9866758wjc.5.2015.04.16.03.50.14 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Apr 2015 03:50:15 -0700 (PDT) From: Thomas Monjalon To: "Butler, Siobhan A" Date: Thu, 16 Apr 2015 12:49:31 +0200 Message-ID: <1754553.FUOz7kZX8l@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <0C5AFCA4B3408848ADF2A3073F7D8CC86D58F9C2@IRSMSX109.ger.corp.intel.com> References: <3571725.20GtF5MAnU@xps13> <0C5AFCA4B3408848ADF2A3073F7D8CC86D58F9C2@IRSMSX109.ger.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] tools brainstorming X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Apr 2015 10:50:18 -0000 2015-04-08 10:43, Butler, Siobhan A: > To add to the tools brainstorming - I propose we use the following Coding > Standards as the basis of guidelines on coding style going forward. Thanks for proposing, it's clearly something which must be written and agreed in coming weeks. I think it will avoid some questions/discussions and will make contributions easier to produce and to review. > The style outlined below is in alignment with the current convention used for > the majority of the project. Yes it's a sane goal. > Any thoughts/suggestions or feedback welcome. > Thanks > Siobhan :) [...] > 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)), URLs may be helpful: https://www.kernel.org/doc/Documentation/CodingStyle https://www.freebsd.org/cgi/man.cgi?query=style&sektion=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. [...] > Indentation should be to no more than 3 levels deep. Useless and too restrictive: line length recommendation should be enough. > 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. [...] > Global pathnames are defined in . Pathnames local to the program go in "pathnames.h" in the local directory. > #include I don't understand. > NOTE Please avoid, as much as possible, including headers from other headers file. This was the old style. Now, the trend is to nest includes in headers to make them self-contained: git grep '#include.*rte' lib | grep 'h:' | sort > Doing so should be properly explained and justified. [...] > Do not ``#define`` or declare names in the implementation namespace except for implementing application interfaces. The namespace should be explicited: why it is prefixed rte_/RTE_ and not dpdk_. > the function name is all in lowercase and the macro has the same name all in uppercase. Right-justify the backslashes; Alignment of backslashes (and alignment in general) is hard to achieve with tab indentation to be nice with different tab sizes. Linux chose to fix tab at 8 spaces. I feel it is too restrictive and I'd prefer avoiding alignments if there are different levels of indentation. > 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. Yes, excellent recommendation. > The comment for #endif should match the expression used in the corresponding #if or #ifdef. Matter of taste: I prefer when the #endif comment match the last condition (#if, #ifdef, #elif or #else). > 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). [...] > 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. Yes :) [...] > In declarations, do not put any whitespace between asterisks and adjacent tokens, except for tokens that are identifiers related to types. > (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. Please elaborate, it's not clear. Using "previous/following" would help. [...] > When declaring variables in structures, declare them sorted by use, then by size (largest to smallest), and then in alphabetical order. Struct sorting is not so important and updates must be done at the end if the struct is part of the API. [...] > It is recommended, but not required that all functions are prototyped somewhere. No, it's simpler to maintain code without local prototypes. Please let's stop useless declarations and let's try to implement functions in the right order. [...] > static void usage(void); static void usage(void); > C Command Line Parsing > ---------------------- Is this section really needed? [...] > Do not add whitespace at the end of a line. Nor blank line at the end of the file. > exit(0); /* > * Avoid obvious comments such as > * "Exit 0 on success." > */ Yes please, and update or remove them when code is updated ;) > When declaring variables in functions, declare them sorted by size, then in alphabetical order. Disagree with such sorting. It should be dependent of the context. Just try to order at your best. [...] > If a line overflows reuse the type keyword. Obviously but not clear when reading. [...] > Logging and Errors > ------------------ [...] > Variable Arguments List > ----------------------- [...] > Printf > ------ [...] > Usage > ----- [...] > Branch Prediction > ----------------- [...] These sections are useless in my opinion. [...] > 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. No more bare-metal. > > 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 Should be avoided. > Use the CONFIG option in the Makefile. Should be avoided. > 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. Most of things must be handled in EAL in specific directories (per-arch, per-OS). [...] > 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. [...] > 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. Do not try to list macros, it is or will be outdated. [...] > Documenting Constants and Variables > /** > * The definition of a funny TRUE. > */ > #define TRUE 0 > > #define TRUE 1 /**< another way to document a macro */ It's also possible to make a comment for a group of macros: /**@{ * Useless macros to make reader confused */ #define TRUE 0 #define SIZE64 32 /**@}*/