Re: [PATCH v6] devtools: add .clang-format file

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 14 Oct 2024 22:15:45 +0000
Abdullah Ömer Yamaç <aomeryamac@gmail.com> 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ç <aomeryamac@gmail.com>
> ---

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.