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

["Abdullah Ömer Yamaç" <aomeryamac@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3480 bytes --]

I want to update you.

On Mon, May 13, 2024 at 4:08 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 5/8/2024 10:19 PM, Abdullah Ömer Yamaç 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>
> >
>
> Hi Omer,
>
> I tried on ethdev.c (clang-format -i ./lib/ethdev/rte_ethdev.c), I will
> highlight a few issues below (not all of them), I hope it is OK to
> continue step by step, fixing these issues.
>
> 1. clang format failed for following options, not sure why, am I using a
> wrong version:
> LineEnding: LF
> InsertNewlineAtEOF: true
>
> I commented them out to continue the test.
>
> And for 'ColumnLimit', I prefer default 80 with the flexibility to go
> 100 when makes sense, so I will got with 'ColumnLimit: 80'; but I don't
> want to start this discussion.
>
> In the .editorconfig file, 100 is stated as a max_line_length. That's why
I prefer 100.

>
> 2. Double tab indentation vs parenthesis align
>          if (iter->bus != NULL &&
>  -                       /* not in middle of rte_eth_dev iteration, */
>  -                       iter->class_device == NULL) {
>  +           /* not in middle of rte_eth_dev iteration, */
>  +           iter->class_device == NULL) {
>
> DPDK coding guide suggests double tab, but also accepts alignment by
> spaces. But as far as I can see most of code has double tab.
> Majority of the diff caused because of this rule.
>
> Still, some discussions are going on

>
> 3. enum merged into single line
>  -enum {
>  -       STAT_QMAP_TX = 0,
>  -       STAT_QMAP_RX
>  -};
>  +enum { STAT_QMAP_TX = 0, STAT_QMAP_RX };
>
> This is ok; I will send a new patch for it.

>
> 4. split message strings
>  - RTE_ETHDEV_LOG_LINE(ERR,
>  -         "Cannot initialize iterator from NULL device description
> string");
>  + RTE_ETHDEV_LOG_LINE(ERR, "Cannot initialize iterator from NULL "
>  +                          "device description string");
>
>
> 5. Empty open parenthesis
>  -       RTE_ETHDEV_LOG_LINE(ERR,
>  -               "Cannot get next device from NULL iterator");
>  +       RTE_ETHDEV_LOG_LINE(
>  +               ERR, "Cannot get next device from NULL iterator");
>
> I couldn't find a solution, so I am still working on it.

>
> 6. space before macro arguments
> -       RTE_ETH_FOREACH_DEV(p)
> +       RTE_ETH_FOREACH_DEV (p)
>
> This is ok. The fix will be in the next patch.

>
> 7. some lists get merged (this seems similar to 3.)
>  -               RTE_PTYPE_L2_MASK,
>  -               RTE_PTYPE_L3_MASK,
>  -               RTE_PTYPE_L4_MASK,
>  -               RTE_PTYPE_TUNNEL_MASK,
>  -               RTE_PTYPE_INNER_L2_MASK,
>  -               RTE_PTYPE_INNER_L3_MASK,
>  +               RTE_PTYPE_L2_MASK,       RTE_PTYPE_L3_MASK,
>  +               RTE_PTYPE_L4_MASK,       RTE_PTYPE_TUNNEL_MASK,
>  +               RTE_PTYPE_INNER_L2_MASK, RTE_PTYPE_INNER_L3_MASK,
>
> I couldn't find a configuration that aligns with array initialization.

>
>
> Thanks,
> ferruh
>

[-- Attachment #2: Type: text/html, Size: 5210 bytes --]