[v4] devtools: add .clang-format file

Message ID 20240508211934.1143124-1-aomeryamac@gmail.com (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series [v4] devtools: add .clang-format file |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/intel-Testing success Testing PASS

Commit Message

Abdullah Ömer Yamaç May 8, 2024, 9:19 p.m. UTC
  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>
---
 .clang-format | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100644 .clang-format
  

Comments

Ferruh Yigit May 13, 2024, 1:08 p.m. UTC | #1
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.


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.


3. enum merged into single line
 -enum {
 -       STAT_QMAP_TX = 0,
 -       STAT_QMAP_RX
 -};
 +enum { STAT_QMAP_TX = 0, STAT_QMAP_RX };


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");


6. space before macro arguments
-       RTE_ETH_FOREACH_DEV(p)
+       RTE_ETH_FOREACH_DEV (p)


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,



Thanks,
ferruh
  
Stephen Hemminger May 13, 2024, 3:55 p.m. UTC | #2
On Mon, 13 May 2024 14:08:07 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> 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.


I personally am more used the aligned style, and most tools support that.
The DPDK one is unique (not done by most other projects). So can we just
keep the kernel (what is this clang-format) version.
  
Morten Brørup May 13, 2024, 7:11 p.m. UTC | #3
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, 13 May 2024 17.55
> 
> On Mon, 13 May 2024 14:08:07 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> 
> > 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.
> 
> 
> I personally am more used the aligned style, and most tools support
> that.
> The DPDK one is unique (not done by most other projects). So can we just
> keep the kernel (what is this clang-format) version.

I personally prefer the double tab.
It also works with editors showing tab as 4 space indentation.

Mixing tabs and spaces only works if the editor shows tabs as 8 space indentation.

Double tab works with both editor configurations.

And there is no confusion if the following block happens to be aligned with the following parameters. E.g.:

if fool(x,
        y)
        myfn();

vs.

if fool(x,
                y)
        myfn();
  
Bruce Richardson May 14, 2024, 7:56 a.m. UTC | #4
On Mon, May 13, 2024 at 09:11:32PM +0200, Morten Brørup wrote:
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Monday, 13 May 2024 17.55
> > 
> > On Mon, 13 May 2024 14:08:07 +0100
> > Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> > 
> > > 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.
> > 
> > 
> > I personally am more used the aligned style, and most tools support
> > that.
> > The DPDK one is unique (not done by most other projects). So can we just
> > keep the kernel (what is this clang-format) version.
> 
> I personally prefer the double tab.
> It also works with editors showing tab as 4 space indentation.
> 
> Mixing tabs and spaces only works if the editor shows tabs as 8 space indentation.
> 
> Double tab works with both editor configurations.
> 
> And there is no confusion if the following block happens to be aligned with the following parameters. E.g.:
> 
> if fool(x,
>         y)
>         myfn();
> 
> vs.
> 
> if fool(x,
>                 y)
>         myfn();
> 

+1, I also prefer the double tab too for this reason. The other
consideration is that double tab leads to smaller diffs on refactor - with
aligning brackets if something on the first line changes it could cause
whitespace changes to be needed on all subsequent lines 

Overall, ignoring our individual preferences, since we already have a mix
in DPDK, I think it's infeasible to try and enforce a single standard now.
:-(

/Bruce
  
Tyler Retzlaff May 14, 2024, 4:59 p.m. UTC | #5
On Tue, May 14, 2024 at 08:56:49AM +0100, Bruce Richardson wrote:
> On Mon, May 13, 2024 at 09:11:32PM +0200, Morten Brørup wrote:
> > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > Sent: Monday, 13 May 2024 17.55
> > > 
> > > On Mon, 13 May 2024 14:08:07 +0100
> > > Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> > > 
> > > > 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.
> > > 
> > > 
> > > I personally am more used the aligned style, and most tools support
> > > that.
> > > The DPDK one is unique (not done by most other projects). So can we just
> > > keep the kernel (what is this clang-format) version.
> > 
> > I personally prefer the double tab.
> > It also works with editors showing tab as 4 space indentation.
> > 
> > Mixing tabs and spaces only works if the editor shows tabs as 8 space indentation.
> > 
> > Double tab works with both editor configurations.
> > 
> > And there is no confusion if the following block happens to be aligned with the following parameters. E.g.:
> > 
> > if fool(x,
> >         y)
> >         myfn();
> > 
> > vs.
> > 
> > if fool(x,
> >                 y)
> >         myfn();
> > 
> 
> +1, I also prefer the double tab too for this reason. The other
> consideration is that double tab leads to smaller diffs on refactor - with
> aligning brackets if something on the first line changes it could cause
> whitespace changes to be needed on all subsequent lines 
> 
> Overall, ignoring our individual preferences, since we already have a mix
> in DPDK, I think it's infeasible to try and enforce a single standard now.
> :-(

-1 to anything that aligns with parameters. it's the most mechanically
inconsistent convention ever, hostile to refactoring and causes a
requirement to endlessly shift your eyes horizontally when reading
code sequentially.
  
Abdullah Ömer Yamaç May 15, 2024, 8:28 a.m. UTC | #6
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
>
  
Bruce Richardson May 15, 2024, 8:43 a.m. UTC | #7
On Wed, May 15, 2024 at 11:28:33AM +0300, Abdullah Ömer Yamaç wrote:
>    I want to update you.
>    On Mon, May 13, 2024 at 4:08 PM Ferruh Yigit <[1]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ç <[2]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.
> 

+1 for keeping as 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
> 

This is one where I don't think we will were reach a consensus, and even if
we did, it would mean massive churn to DPDK. Can we have clang-format NOT
adjust line-continuations in a file?

Thanks,
/Bruce
  
Abdullah Ömer Yamaç May 15, 2024, 10:19 a.m. UTC | #8
On Wed, May 15, 2024 at 11:43 AM Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Wed, May 15, 2024 at 11:28:33AM +0300, Abdullah Ömer Yamaç wrote:
> >    I want to update you.
> >    On Mon, May 13, 2024 at 4:08 PM Ferruh Yigit <[1]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ç <[2]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.
> >
>
> +1 for keeping as 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
> >
>
> This is one where I don't think we will were reach a consensus, and even if
> we did, it would mean massive churn to DPDK. Can we have clang-format NOT
> adjust line-continuations in a file?
>
> I am not an expert on clang, but it seems we don't have such a
configuration.
There is an option called "AlignAfterOpenBracket" which horizontally aligns
arguments after an open bracket.
if I set it to "DontAlign" then "ContinuationIndentWidth" will be active.
Depending on the value of ContinuationIndentWidth, two tabs, single tabs,
or spaces will be acceptable.

> Thanks,
> /Bruce
>
  
Bruce Richardson May 15, 2024, 11:41 a.m. UTC | #9
On Wed, May 15, 2024 at 01:19:50PM +0300, Abdullah Ömer Yamaç wrote:
>    On Wed, May 15, 2024 at 11:43 AM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Wed, May 15, 2024 at 11:28:33AM +0300, Abdullah Ömer Yamaç wrote:
>      >    I want to update you.
>      >    On Mon, May 13, 2024 at 4:08 PM Ferruh Yigit
>      <[1][2]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ç
>      <[2][3]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.
>      >
>      +1 for keeping as 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
>      >
>      This is one where I don't think we will were reach a consensus, and
>      even if
>      we did, it would mean massive churn to DPDK. Can we have
>      clang-format NOT
>      adjust line-continuations in a file?
> 
>    I am not an expert on clang, but it seems we don't have such a
>    configuration.
>    There is an option called "AlignAfterOpenBracket" which horizontally
>    aligns arguments after an open bracket.
>    if I set it to "DontAlign" then "ContinuationIndentWidth" will be
>    active. Depending on the value of ContinuationIndentWidth, two tabs,
>    single tabs, or spaces will be acceptable.
> 

Is there any way to specify per-file the style to be used, e.g. via a
header in the file or a macro to override? While the whole DPDK project is
inconsistent, we should look for consistency within files.

In the absence of that, given the number of strong opinions already
expressed, I'd suggest defaulting the align value to DontAlign for now.

/Bruce
  
Stephen Hemminger May 15, 2024, 3:07 p.m. UTC | #10
On Wed, 15 May 2024 09:43:22 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, May 15, 2024 at 11:28:33AM +0300, Abdullah Ömer Yamaç wrote:
> >    I want to update you.
> >    On Mon, May 13, 2024 at 4:08 PM Ferruh Yigit <[1]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ç <[2]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.
> >   
> 
> +1 for keeping as 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
> >   
> 
> This is one where I don't think we will were reach a consensus, and even if
> we did, it would mean massive churn to DPDK. Can we have clang-format NOT
> adjust line-continuations in a file?


Clang format is useful on new and seriously broken code.
Do not want to do it automatically or accept patches across all the current code.

For indentation, can we get some setting that matches what DPDK double tab does?
If not is there something close. Often useful to look at the other clang format
pre-set styles in source.
  
Abdullah Ömer Yamaç May 15, 2024, 8:32 p.m. UTC | #11
On Wed, May 15, 2024 at 6:07 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Wed, 15 May 2024 09:43:22 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
>
> > On Wed, May 15, 2024 at 11:28:33AM +0300, Abdullah Ömer Yamaç wrote:
> > >    I want to update you.
> > >    On Mon, May 13, 2024 at 4:08 PM Ferruh Yigit <[1]
> 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ç <[2]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.
> > >
> >
> > +1 for keeping as 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
> > >
> >
> > This is one where I don't think we will were reach a consensus, and even
> if
> > we did, it would mean massive churn to DPDK. Can we have clang-format NOT
> > adjust line-continuations in a file?
>
>
> Clang format is useful on new and seriously broken code.
> Do not want to do it automatically or accept patches across all the
> current code.
>
> For indentation, can we get some setting that matches what DPDK double tab
> does?
>
It seems possible. If there is no objection, I will send a new patch with
the previous notes Ferruh stated.

> If not is there something close. Often useful to look at the other clang
> format
> pre-set styles in source.
>
  

Patch

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..6d270524f7
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,141 @@ 
+---
+BasedOnStyle: LLVM
+
+# Place opening and closing parentheses on the same line for control statements
+BreakBeforeBraces: Custom
+BraceWrapping:
+        AfterFunction: true
+        AfterControlStatement: false
+
+# Set maximum line length to 100 characters
+ColumnLimit: 100
+
+# Use LF (line feed) as the end-of-line character
+LineEnding: LF
+
+# Insert a newline at the end of the file
+InsertNewlineAtEOF: true
+
+# Set indentation width to 8 spaces
+IndentWidth: 8
+
+# Set continuation indentation width to 8 spaces
+ContinuationIndentWidth: 8
+
+# Set tab width to 8 spaces
+TabWidth: 8
+
+# Use tabs for indentation
+UseTab: Always
+
+# Preserve include blocks as they are
+IncludeBlocks: Preserve
+
+# Never sort includes
+SortIncludes: Never
+
+# Always break after return type for top-level definitions
+AlwaysBreakAfterReturnType: TopLevelDefinitions
+
+# Always break before multiline string literals
+AlignEscapedNewlines: Left
+
+# Foreach macros
+ForEachMacros:
+        [
+                "CIRBUF_FOREACH",
+                "DLB2_LIST_FOR_EACH",
+                "DLB2_LIST_FOR_EACH_SAFE",
+                "ECORE_LIST_FOR_EACH_ENTRY",
+                "ECORE_LIST_FOR_EACH_ENTRY_SAFE",
+                "FOR_EACH",
+                "FOR_EACH_BUCKET",
+                "FOR_EACH_CNIC_QUEUE",
+                "FOR_EACH_COS_IN_TX_QUEUE",
+                "FOR_EACH_ETH_QUEUE",
+                "FOR_EACH_MEMBER",
+                "FOR_EACH_NONDEFAULT_ETH_QUEUE",
+                "FOR_EACH_NONDEFAULT_QUEUE",
+                "FOR_EACH_PORT",
+                "FOR_EACH_PORT_IF",
+                "FOR_EACH_QUEUE",
+                "FOR_EACH_SUITE_TESTCASE",
+                "FOR_EACH_SUITE_TESTSUITE",
+                "FOREACH_ABS_FUNC_IN_PORT",
+                "FOREACH_DEVICE_ON_AUXILIARY_BUS",
+                "FOREACH_DEVICE_ON_CDXBUS",
+                "FOREACH_DEVICE_ON_PCIBUS",
+                "FOREACH_DEVICE_ON_PLATFORM_BUS",
+                "FOREACH_DEVICE_ON_UACCEBUS",
+                "FOREACH_DEVICE_ON_VMBUS",
+                "FOREACH_DRIVER_ON_AUXILIARY_BUS",
+                "FOREACH_DRIVER_ON_CDXBUS",
+                "FOREACH_DRIVER_ON_PCIBUS",
+                "FOREACH_DRIVER_ON_PLATFORM_BUS",
+                "FOREACH_DRIVER_ON_UACCEBUS",
+                "FOREACH_DRIVER_ON_VMBUS",
+                "FOREACH_SUBDEV",
+                "FOREACH_SUBDEV_STATE",
+                "HLIST_FOR_EACH_ENTRY",
+                "ILIST_FOREACH",
+                "LIST_FOR_EACH_ENTRY",
+                "LIST_FOR_EACH_ENTRY_SAFE",
+                "LIST_FOREACH",
+                "LIST_FOREACH_FROM",
+                "LIST_FOREACH_FROM_SAFE",
+                "LIST_FOREACH_SAFE",
+                "ML_AVG_FOREACH_QP",
+                "ML_AVG_FOREACH_QP_MVTVM",
+                "ML_AVG_RESET_FOREACH_QP",
+                "ML_MAX_FOREACH_QP",
+                "ML_MAX_FOREACH_QP_MVTVM",
+                "ML_MAX_RESET_FOREACH_QP",
+                "ML_MIN_FOREACH_QP",
+                "ML_MIN_FOREACH_QP_MVTVM",
+                "ML_MIN_RESET_FOREACH_QP",
+                "MLX5_ETH_FOREACH_DEV",
+                "MLX5_IPOOL_FOREACH",
+                "MLX5_L3T_FOREACH",
+                "OSAL_LIST_FOR_EACH_ENTRY",
+                "OSAL_LIST_FOR_EACH_ENTRY_SAFE",
+                "PLT_TAILQ_FOREACH_SAFE",
+                "RTE_BBDEV_FOREACH",
+                "RTE_DEV_FOREACH",
+                "RTE_DMA_FOREACH_DEV",
+                "RTE_EAL_DEVARGS_FOREACH",
+                "RTE_ETH_FOREACH_DEV",
+                "RTE_ETH_FOREACH_DEV_OF",
+                "RTE_ETH_FOREACH_DEV_OWNED_BY",
+                "RTE_ETH_FOREACH_DEV_SIBLING",
+                "RTE_ETH_FOREACH_MATCHING_DEV",
+                "RTE_ETH_FOREACH_VALID_DEV",
+                "RTE_GPU_FOREACH",
+                "RTE_GPU_FOREACH_CHILD",
+                "RTE_GPU_FOREACH_PARENT",
+                "RTE_LCORE_FOREACH",
+                "RTE_LCORE_FOREACH_WORKER",
+                "RTE_TAILQ_FOREACH",
+                "RTE_TAILQ_FOREACH_SAFE",
+                "SILIST_FOREACH",
+                "SLIST_FOREACH",
+                "SLIST_FOREACH_FROM",
+                "SLIST_FOREACH_FROM_SAFE",
+                "SLIST_FOREACH_PREVPTR",
+                "SLIST_FOREACH_SAFE",
+                "STAILQ_FOREACH",
+                "STAILQ_FOREACH_FROM",
+                "STAILQ_FOREACH_FROM_SAFE",
+                "STAILQ_FOREACH_SAFE",
+                "TAILQ_FOREACH",
+                "TAILQ_FOREACH_ENTRY",
+                "TAILQ_FOREACH_ENTRY_SAFE",
+                "TAILQ_FOREACH_FROM",
+                "TAILQ_FOREACH_FROM_SAFE",
+                "TAILQ_FOREACH_REVERSE",
+                "TAILQ_FOREACH_REVERSE_FROM",
+                "TAILQ_FOREACH_REVERSE_FROM_SAFE",
+                "TAILQ_FOREACH_REVERSE_SAFE",
+                "TAILQ_FOREACH_SAFE",
+        ]
+ObjCSpaceAfterProperty: true
+IndentGotoLabels: false