On Mon, 14 Oct 2024 22:15:45 +0000 Abdullah Ömer Yamaç <[email protected]> wrote:
> clang-format is a tool to format C/C++/Objective-C code. It can be used > to reformat code to match a given coding style, or to ensure that code > adheres to a specific coding style. It helps to maintain a consistent > coding style across the DPDK codebase. > > .clang-format file overrides the default style options provided by > clang-format and large set of IDEs and text editors support it. > > Signed-off-by: Abdullah Ömer Yamaç <[email protected]> > --- Adding a clang-format would be good, but the patch never got reviewed. I was a little concerned that the new clang-format would have some style conflicts with current DPDK usage; so I asked AI. Please consider these proposed changes and resubmit. ## Patch Review: devtools: add .clang-format file ### Commit Message Analysis | Check | Status | Notes | |-------|--------|-------| | Subject ≤60 chars | ✅ | 39 characters | | Lowercase after colon | ✅ | "add .clang-format file" | | Imperative mood | ✅ | "add" | | No trailing period | ✅ | | | Signed-off-by | ✅ | Present with real name and email | | Body ≤75 chars | ✅ | Lines are within limit | **Warning**: The prefix `devtools:` is potentially misleading since the `.clang-format` file is being added to the repository root, not to the `devtools/` directory. Consider using a prefix like `style:` or `build:`, or simply omit the prefix for root-level config files. --- ### Clang-Format Settings Review #### ✅ Correct Settings | Setting | Value | DPDK Requirement | Status | |---------|-------|------------------|--------| | `ColumnLimit` | 100 | 100 chars max | ✅ | | `TabWidth` | 8 | 8 characters | ✅ | | `IndentWidth` | 8 | 8 characters | ✅ | | `LineEnding` | LF | Unix line endings | ✅ | | `InsertNewlineAtEOF` | true | Files end with newline | ✅ | | `AfterFunction` | true | Brace on new line for functions | ✅ | | `AfterControlStatement` | false | Brace on same line for if/for/while | ✅ | | `IncludeBlocks` | Preserve | Don't reorder include groups | ✅ | | `SortIncludes` | Never | Preserve include order | ✅ | | `AlwaysBreakAfterReturnType` | TopLevelDefinitions | Return type on own line | ✅ | | `IndentGotoLabels` | false | Labels not indented | ✅ | | `ForEachMacros` | comprehensive list | Treat as loops | ✅ | #### ⚠️ Issues to Address **1. UseTab Setting (Warning)** ```yaml UseTab: Always ``` DPDK style specifies: *"hard tabs for indentation, spaces for alignment"* `UseTab: Always` will use tabs everywhere including for alignment, which can cause misalignment when viewed with different tab widths. Consider: ```yaml UseTab: ForIndentation ``` **2. Missing Pointer Alignment (Warning)** DPDK follows Linux kernel style where the `*` binds to the variable name: ```c char *ptr; /* Correct */ char* ptr; /* Wrong */ ``` Add: ```yaml PointerAlignment: Right ``` **3. Missing Space After Cast (Info)** DPDK style typically has no space after casts. Consider adding: ```yaml SpaceAfterCStyleCast: false ``` **4. Missing Short Function Settings (Info)** To prevent unwanted single-line functions: ```yaml AllowShortFunctionsOnASingleLine: None AllowShortBlocksOnASingleLine: Never ``` **5. Missing Brace Wrapping Settings (Info)** The `BraceWrapping` section is incomplete. For full DPDK compliance, consider: ```yaml BraceWrapping: AfterFunction: true AfterControlStatement: false AfterStruct: false AfterEnum: false AfterUnion: false BeforeElse: false BeforeWhile: false SplitEmptyFunction: true ``` **6. Switch Statement Indentation (Info)** DPDK style shows `case` labels not indented relative to `switch`. Add: ```yaml IndentCaseLabels: false IndentCaseBlocks: false ``` **7. BasedOnStyle Consideration (Info)** `BasedOnStyle: LLVM` is a reasonable starting point, but Linux kernel style (`BasedOnStyle: Kernel` if available in your clang-format version) might be closer to DPDK's actual style since DPDK is modeled on Linux kernel development. --- ### Summary | Severity | Count | Items | |----------|-------|-------| | **Error** | 0 | None | | **Warning** | 2 | `UseTab: Always` should be `ForIndentation`; Missing `PointerAlignment: Right` | | **Info** | 5 | Subject prefix; Missing short function settings; Incomplete brace wrapping; Missing switch indent settings; Space after cast | **Recommendation**: The patch is a solid foundation but needs the pointer alignment and tab usage fixes before merge. The current `UseTab: Always` setting will cause alignment issues in multi-line function arguments and struct initializers, and the missing `PointerAlignment: Right` will format pointers contrary to DPDK convention.
