[PATCH] devtools: add .clang-format file

[PATCH] devtools: add .clang-format file

[Abdullah Ömer Yamaç]

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 | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 .clang-format

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..8ac10c0c09
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,30 @@
+---
+# 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
+EndOfLine: lf
+
+# Specify style options for the entire file
+File:
+        # Insert a final newline character at the end of the file
+        EndOfFile: true
+
+IndentWidth: 8
+
+# Set tab width to 8 spaces
+TabWidth: 8
+
+# Use tabs for indentation
+UseTab: Always
+
+# Specify whitespace-related style options
+Whitespace:
+        # Trim trailing whitespace at the end of each line
+        TrimTrailingWhitespace: true
-- 
2.34.1

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

[Ferruh Yigit]

On 4/29/2024 2:04 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>
>

Overall +1 to have a format file, specially to help non-frequent
contributors.

1. Some options are failing for me [1], I don't know if it requires a
specific version of clang-format

2. Current options are not fully aligned with coding convention, it is
easier to see what is not compatible by running the tool on an existing
file [2].
we need to fix them.



[1]
`clang-format -i ./lib/ethdev/rte_ethdev.c`
a.
/home/amd/development/dpdk-next-net/./.clang-format:28:1: error: unknown
key 'Whitespace'
Whitespace:
^~~~~~~~~~
Error reading /home/amd/development/dpdk-next-net/./.clang-format:
Invalid argument

b.
/home/amd/development/dpdk-next-net/./.clang-format:12:1: error: unknown
key 'EndOfLine'
EndOfLine: lf
^~~~~~~~~
Error reading /home/amd/development/dpdk-next-net/./.clang-format:
Invalid argument

c.
/home/amd/development/dpdk-next-net/./.clang-format:15:1: error: unknown
key 'File'
File:
^~~~
Error reading /home/amd/development/dpdk-next-net/./.clang-format:
Invalid argument



[2]
`clang-format -i ./lib/ethdev/rte_ethdev.c` (with failing part commented
out)
`git diff`

A few diff samples to fix at first glance:
```
 static const struct rte_eth_xstats_name_off eth_dev_txq_stats_strings[] = {
 -       {"packets", offsetof(struct rte_eth_stats, q_opackets)},
 -       {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
 +    {"packets", offsetof(struct rte_eth_stats, q_opackets)},
 +    {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
 };
```

```
 -enum {
 -       STAT_QMAP_TX = 0,
 -       STAT_QMAP_RX
 -};
 +enum { STAT_QMAP_TX = 0, STAT_QMAP_RX };
```

```
 -int
 -rte_eth_iterator_init(struct rte_dev_iterator *iter, const char
*devargs_str)
 +int rte_eth_iterator_init(struct rte_dev_iterator *iter, const char
*devargs_str)
```

```
         RTE_ETH_FOREACH_DEV(p)
 -               count++;
 +       count++;
```

```
         rte_spinlock_lock(rte_mcfg_ethdev_get_lock());
 -       RTE_ETH_FOREACH_VALID_DEV(pid) {
 +       RTE_ETH_FOREACH_VALID_DEV(pid)
 +       {
```

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

[Stephen Hemminger]

On Mon, 29 Apr 2024 14:32:43 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> ```
>          RTE_ETH_FOREACH_DEV(p)

I think all those foreach macros need to be in clang format.

For example the kernel clang format has:


# Taken from:
#   git grep -h '^#define [^[:space:]]*for_each[^[:space:]]*(' include/ tools/ \
#   | sed "s,^#define \([^[:space:]]*for_each[^[:space:]]*\)(.*$,  - '\1'," \
#   | LC_ALL=C sort -u
ForEachMacros:
  - '__ata_qc_for_each'
  - '__bio_for_each_bvec'
  - '__bio_for_each_segment'
  - '__evlist__for_each_entry'
  - '__evlist__for_each_entry_continue'
  - '__evlist__for_each_entry_from'
  - '__evlist__for_each_entry_reverse'
  - '__evlist__for_each_entry_safe'
  - '__for_each_mem_range'
  - '__for_each_mem_range_rev'
  - '__for_each_thread'
  - '__hlist_for_each_rcu'
  - '__map__for_each_symbol_by_name'
  - '__pci_bus_for_each_res0'
  - '__pci_bus_for_each_res1'
  - '__pci_dev_for_each_res0'
  - '__pci_dev_for_each_res1'
  - '__perf_evlist__for_each_entry'
  - '__perf_evlist__for_each_entry_reverse'
  - '__perf_evlist__for_each_entry_safe'
  - '__rq_for_each_bio'
  - '__shost_for_each_device'
  - '__sym_for_each'
  - 'apei_estatus_for_each_section'
  - 'ata_for_each_dev'
  - 'ata_for_each_link'
  - 'ata_qc_for_each'
  - 'ata_qc_for_each_raw'
  - 'ata_qc_for_each_with_internal'
  - 'ax25_for_each'
  - 'ax25_uid_for_each'
  - 'bio_for_each_bvec'
  - 'bio_for_each_bvec_all'
  - 'bio_for_each_folio_all'
  - 'bio_for_each_integrity_vec'
  - 'bio_for_each_segment'
  - 'bio_for_each_segment_all'
  - 'bio_list_for_each'
  - 'bip_for_each_vec'
  - 'bond_for_each_slave'
  - 'bond_for_each_slave_rcu'
  - 'bpf_for_each'
  - 'bpf_for_each_reg_in_vstate'
  - 'bpf_for_each_reg_in_vstate_mask'
  - 'bpf_for_each_spilled_reg'
  - 'bpf_object__for_each_map'
  - 'bpf_object__for_each_program'
  - 'btree_for_each_safe128'
  - 'btree_for_each_safe32'
  - 'btree_for_each_safe64'
  - 'btree_for_each_safel'
  - 'card_for_each_dev'
  - 'cgroup_taskset_for_each'
  - 'cgroup_taskset_for_each_leader'
  - 'cpu_aggr_map__for_each_idx'
  - 'cpufreq_for_each_efficient_entry_idx'
  - 'cpufreq_for_each_entry'
  - 'cpufreq_for_each_entry_idx'
  - 'cpufreq_for_each_valid_entry'
  - 'cpufreq_for_each_valid_entry_idx'
  - 'css_for_each_child'
  - 'css_for_each_descendant_post'
  - 'css_for_each_descendant_pre'
  - 'damon_for_each_region'
  - 'damon_for_each_region_from'
  - 'damon_for_each_region_safe'
  - 'damon_for_each_scheme'
  - 'damon_for_each_scheme_safe'
  - 'damon_for_each_target'
  - 'damon_for_each_target_safe'
  - 'damos_for_each_filter'
  - 'damos_for_each_filter_safe'
  - 'data__for_each_file'
  - 'data__for_each_file_new'
  - 'data__for_each_file_start'
  - 'device_for_each_child_node'
  - 'displayid_iter_for_each'
  - 'dma_fence_array_for_each'
  - 'dma_fence_chain_for_each'
  - 'dma_fence_unwrap_for_each'
  - 'dma_resv_for_each_fence'
  - 'dma_resv_for_each_fence_unlocked'
  - 'do_for_each_ftrace_op'
  - 'drm_atomic_crtc_for_each_plane'
  - 'drm_atomic_crtc_state_for_each_plane'
  - 'drm_atomic_crtc_state_for_each_plane_state'
  - 'drm_atomic_for_each_plane_damage'
  - 'drm_client_for_each_connector_iter'
  - 'drm_client_for_each_modeset'
  - 'drm_connector_for_each_possible_encoder'
  - 'drm_exec_for_each_locked_object'
  - 'drm_exec_for_each_locked_object_reverse'
  - 'drm_for_each_bridge_in_chain'
  - 'drm_for_each_connector_iter'
  - 'drm_for_each_crtc'
  - 'drm_for_each_crtc_reverse'
  - 'drm_for_each_encoder'
  - 'drm_for_each_encoder_mask'
  - 'drm_for_each_fb'
  - 'drm_for_each_legacy_plane'
  - 'drm_for_each_plane'
  - 'drm_for_each_plane_mask'
  - 'drm_for_each_privobj'
  - 'drm_gem_for_each_gpuva'
  - 'drm_gem_for_each_gpuva_safe'
  - 'drm_gpuva_for_each_op'
  - 'drm_gpuva_for_each_op_from_reverse'
  - 'drm_gpuva_for_each_op_safe'
  - 'drm_gpuvm_for_each_va'
  - 'drm_gpuvm_for_each_va_range'
  - 'drm_gpuvm_for_each_va_range_safe'
  - 'drm_gpuvm_for_each_va_safe'
  - 'drm_mm_for_each_hole'
  - 'drm_mm_for_each_node'
  - 'drm_mm_for_each_node_in_range'
  - 'drm_mm_for_each_node_safe'
  - 'dsa_switch_for_each_available_port'
  - 'dsa_switch_for_each_cpu_port'
  - 'dsa_switch_for_each_cpu_port_continue_reverse'
  - 'dsa_switch_for_each_port'
  - 'dsa_switch_for_each_port_continue_reverse'
  - 'dsa_switch_for_each_port_safe'
  - 'dsa_switch_for_each_user_port'
  - 'dsa_tree_for_each_cpu_port'
  - 'dsa_tree_for_each_user_port'
  - 'dsa_tree_for_each_user_port_continue_reverse'
  - 'dso__for_each_symbol'
  - 'dsos__for_each_with_build_id'
  - 'elf_hash_for_each_possible'
  - 'elf_symtab__for_each_symbol'
  - 'evlist__for_each_cpu'
  - 'evlist__for_each_entry'
  - 'evlist__for_each_entry_continue'
  - 'evlist__for_each_entry_from'
  - 'evlist__for_each_entry_reverse'
  - 'evlist__for_each_entry_safe'
  - 'flow_action_for_each'
  - 'for_each_acpi_consumer_dev'
  - 'for_each_acpi_dev_match'
  - 'for_each_active_dev_scope'
  - 'for_each_active_drhd_unit'
  - 'for_each_active_iommu'
  - 'for_each_active_route'
  - 'for_each_aggr_pgid'
  - 'for_each_and_bit'
  - 'for_each_andnot_bit'
  - 'for_each_available_child_of_node'
  - 'for_each_bench'
  - 'for_each_bio'
  - 'for_each_board_func_rsrc'
  - 'for_each_btf_ext_rec'
  - 'for_each_btf_ext_sec'
  - 'for_each_bvec'
  - 'for_each_card_auxs'
  - 'for_each_card_auxs_safe'
  - 'for_each_card_components'
  - 'for_each_card_dapms'
  - 'for_each_card_pre_auxs'
  - 'for_each_card_prelinks'
  - 'for_each_card_rtds'
  - 'for_each_card_rtds_safe'
  - 'for_each_card_widgets'
  - 'for_each_card_widgets_safe'
  - 'for_each_cgroup_storage_type'
  - 'for_each_child_of_node'
  - 'for_each_clear_bit'
  - 'for_each_clear_bit_from'
  - 'for_each_clear_bitrange'
  - 'for_each_clear_bitrange_from'
  - 'for_each_cmd'
  - 'for_each_cmsghdr'
  - 'for_each_collection'
  - 'for_each_comp_order'
  - 'for_each_compatible_node'
  - 'for_each_component_dais'
  - 'for_each_component_dais_safe'
  - 'for_each_conduit'
  - 'for_each_console'
  - 'for_each_console_srcu'
  - 'for_each_cpu'
  - 'for_each_cpu_and'
  - 'for_each_cpu_andnot'
  - 'for_each_cpu_or'
  - 'for_each_cpu_wrap'
  - 'for_each_dapm_widgets'
  - 'for_each_dedup_cand'
  - 'for_each_dev_addr'
  - 'for_each_dev_scope'
  - 'for_each_dma_cap_mask'
  - 'for_each_dpcm_be'
  - 'for_each_dpcm_be_rollback'
  - 'for_each_dpcm_be_safe'
  - 'for_each_dpcm_fe'
  - 'for_each_drhd_unit'
  - 'for_each_dss_dev'
  - 'for_each_efi_memory_desc'
  - 'for_each_efi_memory_desc_in_map'
  - 'for_each_element'
  - 'for_each_element_extid'
  - 'for_each_element_id'
  - 'for_each_endpoint_of_node'
  - 'for_each_event'
  - 'for_each_event_tps'
  - 'for_each_evictable_lru'
  - 'for_each_fib6_node_rt_rcu'
  - 'for_each_fib6_walker_rt'
  - 'for_each_free_mem_pfn_range_in_zone'
  - 'for_each_free_mem_pfn_range_in_zone_from'
  - 'for_each_free_mem_range'
  - 'for_each_free_mem_range_reverse'
  - 'for_each_func_rsrc'
  - 'for_each_gpiochip_node'
  - 'for_each_group_evsel'
  - 'for_each_group_evsel_head'
  - 'for_each_group_member'
  - 'for_each_group_member_head'
  - 'for_each_hstate'
  - 'for_each_if'
  - 'for_each_inject_fn'
  - 'for_each_insn'
  - 'for_each_insn_prefix'
  - 'for_each_intid'
  - 'for_each_iommu'
  - 'for_each_ip_tunnel_rcu'
  - 'for_each_irq_nr'
  - 'for_each_lang'
  - 'for_each_link_codecs'
  - 'for_each_link_cpus'
  - 'for_each_link_platforms'
  - 'for_each_lru'
  - 'for_each_matching_node'
  - 'for_each_matching_node_and_match'
  - 'for_each_media_entity_data_link'
  - 'for_each_mem_pfn_range'
  - 'for_each_mem_range'
  - 'for_each_mem_range_rev'
  - 'for_each_mem_region'
  - 'for_each_member'
  - 'for_each_memory'
  - 'for_each_migratetype_order'
  - 'for_each_missing_reg'
  - 'for_each_mle_subelement'
  - 'for_each_mod_mem_type'
  - 'for_each_net'
  - 'for_each_net_continue_reverse'
  - 'for_each_net_rcu'
  - 'for_each_netdev'
  - 'for_each_netdev_continue'
  - 'for_each_netdev_continue_rcu'
  - 'for_each_netdev_continue_reverse'
  - 'for_each_netdev_dump'
  - 'for_each_netdev_feature'
  - 'for_each_netdev_in_bond_rcu'
  - 'for_each_netdev_rcu'
  - 'for_each_netdev_reverse'
  - 'for_each_netdev_safe'
  - 'for_each_new_connector_in_state'
  - 'for_each_new_crtc_in_state'
  - 'for_each_new_mst_mgr_in_state'
  - 'for_each_new_plane_in_state'
  - 'for_each_new_plane_in_state_reverse'
  - 'for_each_new_private_obj_in_state'
  - 'for_each_new_reg'
  - 'for_each_node'
  - 'for_each_node_by_name'
  - 'for_each_node_by_type'
  - 'for_each_node_mask'
  - 'for_each_node_state'
  - 'for_each_node_with_cpus'
  - 'for_each_node_with_property'
  - 'for_each_nonreserved_multicast_dest_pgid'
  - 'for_each_numa_hop_mask'
  - 'for_each_of_allnodes'
  - 'for_each_of_allnodes_from'
  - 'for_each_of_cpu_node'
  - 'for_each_of_pci_range'
  - 'for_each_old_connector_in_state'
  - 'for_each_old_crtc_in_state'
  - 'for_each_old_mst_mgr_in_state'
  - 'for_each_old_plane_in_state'
  - 'for_each_old_private_obj_in_state'
  - 'for_each_oldnew_connector_in_state'
  - 'for_each_oldnew_crtc_in_state'
  - 'for_each_oldnew_mst_mgr_in_state'
  - 'for_each_oldnew_plane_in_state'
  - 'for_each_oldnew_plane_in_state_reverse'
  - 'for_each_oldnew_private_obj_in_state'
  - 'for_each_online_cpu'
  - 'for_each_online_node'
  - 'for_each_online_pgdat'
  - 'for_each_or_bit'
  - 'for_each_path'
  - 'for_each_pci_bridge'
  - 'for_each_pci_dev'
  - 'for_each_pcm_streams'
  - 'for_each_physmem_range'
  - 'for_each_populated_zone'
  - 'for_each_possible_cpu'
  - 'for_each_present_blessed_reg'
  - 'for_each_present_cpu'
  - 'for_each_prime_number'
  - 'for_each_prime_number_from'
  - 'for_each_probe_cache_entry'
  - 'for_each_process'
  - 'for_each_process_thread'
  - 'for_each_prop_codec_conf'
  - 'for_each_prop_dai_codec'
  - 'for_each_prop_dai_cpu'
  - 'for_each_prop_dlc_codecs'
  - 'for_each_prop_dlc_cpus'
  - 'for_each_prop_dlc_platforms'
  - 'for_each_property_of_node'
  - 'for_each_reg'
  - 'for_each_reg_filtered'
  - 'for_each_reloc'
  - 'for_each_reloc_from'
  - 'for_each_requested_gpio'
  - 'for_each_requested_gpio_in_range'
  - 'for_each_reserved_mem_range'
  - 'for_each_reserved_mem_region'
  - 'for_each_rtd_codec_dais'
  - 'for_each_rtd_components'
  - 'for_each_rtd_cpu_dais'
  - 'for_each_rtd_dais'
  - 'for_each_sband_iftype_data'
  - 'for_each_script'
  - 'for_each_sec'
  - 'for_each_set_bit'
  - 'for_each_set_bit_from'
  - 'for_each_set_bit_wrap'
  - 'for_each_set_bitrange'
  - 'for_each_set_bitrange_from'
  - 'for_each_set_clump8'
  - 'for_each_sg'
  - 'for_each_sg_dma_page'
  - 'for_each_sg_page'
  - 'for_each_sgtable_dma_page'
  - 'for_each_sgtable_dma_sg'
  - 'for_each_sgtable_page'
  - 'for_each_sgtable_sg'
  - 'for_each_sibling_event'
  - 'for_each_sta_active_link'
  - 'for_each_subelement'
  - 'for_each_subelement_extid'
  - 'for_each_subelement_id'
  - 'for_each_sublist'
  - 'for_each_subsystem'
  - 'for_each_supported_activate_fn'
  - 'for_each_supported_inject_fn'
  - 'for_each_sym'
  - 'for_each_test'
  - 'for_each_thread'

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

[Stephen Hemminger]

On Mon, 29 Apr 2024 14:32:43 +0100
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 4/29/2024 2:04 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>
> >  
> 
> Overall +1 to have a format file, specially to help non-frequent
> contributors.
> 
> 1. Some options are failing for me [1], I don't know if it requires a
> specific version of clang-format
> 
> 2. Current options are not fully aligned with coding convention, it is
> easier to see what is not compatible by running the tool on an existing
> file [2].
> we need to fix them.
> 
> 
> 
> [1]
> `clang-format -i ./lib/ethdev/rte_ethdev.c`
> a.
> /home/amd/development/dpdk-next-net/./.clang-format:28:1: error: unknown
> key 'Whitespace'
> Whitespace:
> ^~~~~~~~~~
> Error reading /home/amd/development/dpdk-next-net/./.clang-format:
> Invalid argument
> 
> b.
> /home/amd/development/dpdk-next-net/./.clang-format:12:1: error: unknown
> key 'EndOfLine'
> EndOfLine: lf
> ^~~~~~~~~
> Error reading /home/amd/development/dpdk-next-net/./.clang-format:
> Invalid argument
> 
> c.
> /home/amd/development/dpdk-next-net/./.clang-format:15:1: error: unknown
> key 'File'
> File:
> ^~~~
> Error reading /home/amd/development/dpdk-next-net/./.clang-format:
> Invalid argument
> 
> 
> 
> [2]
> `clang-format -i ./lib/ethdev/rte_ethdev.c` (with failing part commented
> out)
> `git diff`
> 
> A few diff samples to fix at first glance:
> ```
>  static const struct rte_eth_xstats_name_off eth_dev_txq_stats_strings[] = {
>  -       {"packets", offsetof(struct rte_eth_stats, q_opackets)},
>  -       {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
>  +    {"packets", offsetof(struct rte_eth_stats, q_opackets)},
>  +    {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
>  };
> ```
> 
> ```
>  -enum {
>  -       STAT_QMAP_TX = 0,
>  -       STAT_QMAP_RX
>  -};
>  +enum { STAT_QMAP_TX = 0, STAT_QMAP_RX };
> ```
> 
> ```
>  -int
>  -rte_eth_iterator_init(struct rte_dev_iterator *iter, const char
> *devargs_str)
>  +int rte_eth_iterator_init(struct rte_dev_iterator *iter, const char
> *devargs_str)
> ```
> 
> ```
>          RTE_ETH_FOREACH_DEV(p)

Add this

# Take from
# 
# git grep -h '^#define [^[:space:]]*FOREACH[^[:space:]]*(' lib/ \
#  | sed "s,^#define \([^[:space:]]*FOREACH[^[:space:]]*\)(.*$,  - '\1'," \
#  | LC_ALL=C sort -u
ForeachMacros: 
 - 'CIRBUF_FOREACH'
  - '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'

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

[Abdullah Ömer Yamaç]

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

Hello,
I am facing a problem while creating the configuration. I am trying to make
a clang-format that formats codes without changing any codebases. Here is
the question:

clang-format related config:
...
BraceWrapping:
        AfterFunction: true
...

If the configuration supports braces after function, then MACRO's formats
fail.

An example of the formatted MACRO is given below:
...
#define RTE_RX_OFFLOAD_BIT2STR(_name)              \
{                                          \
...

If the AfterFunction is false, the functions' formats fail; here is a
formatted example.
...
 if (devargs_str == NULL) {
...

I will send the current configuration as v2. You can test it.



On Mon, Apr 29, 2024 at 6:43 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Mon, 29 Apr 2024 14:32:43 +0100
> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> > On 4/29/2024 2:04 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>
> > >
> >
> > Overall +1 to have a format file, specially to help non-frequent
> > contributors.
> >
> > 1. Some options are failing for me [1], I don't know if it requires a
> > specific version of clang-format
> >
> > 2. Current options are not fully aligned with coding convention, it is
> > easier to see what is not compatible by running the tool on an existing
> > file [2].
> > we need to fix them.
> >
> >
> >
> > [1]
> > `clang-format -i ./lib/ethdev/rte_ethdev.c`
> > a.
> > /home/amd/development/dpdk-next-net/./.clang-format:28:1: error: unknown
> > key 'Whitespace'
> > Whitespace:
> > ^~~~~~~~~~
> > Error reading /home/amd/development/dpdk-next-net/./.clang-format:
> > Invalid argument
> >
> > b.
> > /home/amd/development/dpdk-next-net/./.clang-format:12:1: error: unknown
> > key 'EndOfLine'
> > EndOfLine: lf
> > ^~~~~~~~~
> > Error reading /home/amd/development/dpdk-next-net/./.clang-format:
> > Invalid argument
> >
> > c.
> > /home/amd/development/dpdk-next-net/./.clang-format:15:1: error: unknown
> > key 'File'
> > File:
> > ^~~~
> > Error reading /home/amd/development/dpdk-next-net/./.clang-format:
> > Invalid argument
> >
> >
> >
> > [2]
> > `clang-format -i ./lib/ethdev/rte_ethdev.c` (with failing part commented
> > out)
> > `git diff`
> >
> > A few diff samples to fix at first glance:
> > ```
> >  static const struct rte_eth_xstats_name_off eth_dev_txq_stats_strings[]
> = {
> >  -       {"packets", offsetof(struct rte_eth_stats, q_opackets)},
> >  -       {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
> >  +    {"packets", offsetof(struct rte_eth_stats, q_opackets)},
> >  +    {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
> >  };
> > ```
> >
> > ```
> >  -enum {
> >  -       STAT_QMAP_TX = 0,
> >  -       STAT_QMAP_RX
> >  -};
> >  +enum { STAT_QMAP_TX = 0, STAT_QMAP_RX };
> > ```
> >
> > ```
> >  -int
> >  -rte_eth_iterator_init(struct rte_dev_iterator *iter, const char
> > *devargs_str)
> >  +int rte_eth_iterator_init(struct rte_dev_iterator *iter, const char
> > *devargs_str)
> > ```
> >
> > ```
> >          RTE_ETH_FOREACH_DEV(p)
>
> Add this
>
> # Take from
> #
> # git grep -h '^#define [^[:space:]]*FOREACH[^[:space:]]*(' lib/ \
> #  | sed "s,^#define \([^[:space:]]*FOREACH[^[:space:]]*\)(.*$,  - '\1'," \
> #  | LC_ALL=C sort -u
> ForeachMacros:
>  - 'CIRBUF_FOREACH'
>   - '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'
>
>
>

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

[PATCH v2] devtools: add .clang-format file

[Abdullah Ömer Yamaç]

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 | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 .clang-format

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..480beaca20
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,42 @@
+---
+BasedOnStyle: LLVM
+
+# Place opening and closing parentheses on the same line for control statements
+BreakBeforeBraces: Custom
+BraceWrapping:
+        AfterFunction: false
+        AfterControlStatement: false
+        AfterEnum: 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
-- 
2.34.1

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

[Stephen Hemminger]

On Sat,  4 May 2024 13:41:35 +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>

You need to add the ForEachMacros: I sent in earlier email

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

[Abdullah Ömer Yamaç]

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

I was confused about the kernel functions. I didn't understand what you
meant at first, but now I understand.  I will send a new patch with all
DPDK-related foreach macros.

On Sat, May 4, 2024 at 7:27 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Sat,  4 May 2024 13:41:35 +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>
>
> You need to add the ForEachMacros: I sent in earlier email
>
>

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

[PATCH v3] devtools: add .clang-format file

[Abdullah Ömer Yamaç]

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 | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 .clang-format

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..16164ef1de
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,138 @@
+---
+BasedOnStyle: LLVM
+
+# Place opening and closing parentheses on the same line for control statements
+BreakBeforeBraces: Custom
+BraceWrapping:
+        AfterFunction: false
+        AfterControlStatement: false
+        AfterEnum: 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", ]
-- 
2.34.1

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

[Stephen Hemminger]

On Sat,  4 May 2024 19:18:37 +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>
> ---
>  .clang-format | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 138 insertions(+)
>  create mode 100644 .clang-format

Tried this, but it needs some change to how braces at start of function
are handled.  For example, this is not how DPDK should look:

 static int
-rte_pmd_tap_remove(struct rte_vdev_device *dev)
-{
+rte_pmd_tap_remove(struct rte_vdev_device *dev) {
 	struct rte_eth_dev *eth_dev = NULL;
 
 	/* find the ethdev entry */

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

[Abdullah Ömer Yamaç]

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

On Sun, May 5, 2024 at 7:18 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Sat,  4 May 2024 19:18:37 +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>
> > ---
> >  .clang-format | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 138 insertions(+)
> >  create mode 100644 .clang-format
>
> Tried this, but it needs some change to how braces at start of function
> are handled.  For example, this is not how DPDK should look:
>
Are the changes below ok? When I fix these cases, some macros are also
formatted in the same manner.

-#define RTE_RX_OFFLOAD_BIT2STR(_name) \
- { RTE_ETH_RX_OFFLOAD_##_name, #_name }
+#define RTE_RX_OFFLOAD_BIT2STR(_name)              \
+ {                                          \
+ RTE_ETH_RX_OFFLOAD_##_name, #_name \
+ }


>
>  static int
> -rte_pmd_tap_remove(struct rte_vdev_device *dev)
> -{
> +rte_pmd_tap_remove(struct rte_vdev_device *dev) {
>         struct rte_eth_dev *eth_dev = NULL;
>
>         /* find the ethdev entry */
>

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

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

[Stephen Hemminger]

On Sat,  4 May 2024 19:18:37 +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>

Also, this looks wrong.  The initialized arrays looked better before.

 
-static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {
-	"UNKNOWN", "TUN", "TAP"
-};
+static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {"UNKNOWN", "TUN", "TAP"};
 
-static const char *valid_arguments[] = {
-	ETH_TAP_IFACE_ARG,
-	ETH_TAP_REMOTE_ARG,
-	ETH_TAP_MAC_ARG,
-	ETH_TAP_PERSIST_ARG,
-	NULL
-};
+static const char *valid_arguments[] = {ETH_TAP_IFACE_ARG, ETH_TAP_REMOTE_ARG, ETH_TAP_MAC_ARG,
+					ETH_TAP_PERSIST_ARG, NULL};

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

[Abdullah Ömer Yamaç]

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

On Sun, May 5, 2024 at 7:21 PM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Sat,  4 May 2024 19:18:37 +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>
>
> Also, this looks wrong.  The initialized arrays looked better before.
>
>
> -static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {
> -       "UNKNOWN", "TUN", "TAP"
> -};
> +static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {"UNKNOWN", "TUN",
> "TAP"};
>
> -static const char *valid_arguments[] = {
> -       ETH_TAP_IFACE_ARG,
> -       ETH_TAP_REMOTE_ARG,
> -       ETH_TAP_MAC_ARG,
> -       ETH_TAP_PERSIST_ARG,
> -       NULL
> -};
> +static const char *valid_arguments[] = {ETH_TAP_IFACE_ARG,
> ETH_TAP_REMOTE_ARG, ETH_TAP_MAC_ARG,
> +                                       ETH_TAP_PERSIST_ARG, NULL};
>

I am confused about these variables.  tuntap_types list values in a single
line, but valid_arguments' values are listed separately.
So, it is getting more complex to handle both of them. What should we do,
do you have any idea?

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

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

[Stephen Hemminger]

On Sun, 5 May 2024 22:42:57 +0300
Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:

> > Also, this looks wrong.  The initialized arrays looked better before.
> >
> >
> > -static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {
> > -       "UNKNOWN", "TUN", "TAP"
> > -};
> > +static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {"UNKNOWN", "TUN",
> > "TAP"};
> >
> > -static const char *valid_arguments[] = {
> > -       ETH_TAP_IFACE_ARG,
> > -       ETH_TAP_REMOTE_ARG,
> > -       ETH_TAP_MAC_ARG,
> > -       ETH_TAP_PERSIST_ARG,
> > -       NULL
> > -};
> > +static const char *valid_arguments[] = {ETH_TAP_IFACE_ARG,
> > ETH_TAP_REMOTE_ARG, ETH_TAP_MAC_ARG,
> > +                                       ETH_TAP_PERSIST_ARG, NULL};
> >  
> 
> I am confused about these variables.  tuntap_types list values in a single
> line, but valid_arguments' values are listed separately.
> So, it is getting more complex to handle both of them. What should we do,
> do you have any idea?

Ignore the initialized lists for now. It should be possible to have it generate something
like 

static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {
      "UNKNOWN", "TUN", "TAP"
};

With the following changes result looks better. You got the format wrong for the ForEach list.

diff --git a/.clang-format b/.clang-format
index 16164ef1de..d16185c049 100644
--- a/.clang-format
+++ b/.clang-format
@@ -1,12 +1,20 @@
 ---
 BasedOnStyle: LLVM
 
+AttributeMacros:
+  - __rte_aligned
+  - __rte_packed
+  - __rte_may_alias
+  - __rte_deprecated
+  - __rte_weak
+  - __rte_unused
+  - __rte_restrict
+
 # Place opening and closing parentheses on the same line for control statements
 BreakBeforeBraces: Custom
 BraceWrapping:
-        AfterFunction: false
+        AfterFunction: true
         AfterControlStatement: false
-        AfterEnum: false
 
 # Set maximum line length to 100 characters
 ColumnLimit: 100
@@ -41,98 +49,117 @@ 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", ]
+ForEachMacros:
+  - CIRBUF_FOREACH
+  - DLB2_LIST_FOR_EACH
+  - DLB2_LIST_FOR_EACH_SAFE
+  - ECORE_LIST_FOR_EACH_ENTRY
+  - ECORE_LIST_FOR_EACH_ENTRY_SAFE
+  - 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
+  - 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
+  - HLIST_FOR_EACH_ENTRY
+  - ILIST_FOREACH
+  - LIST_FOREACH
+  - LIST_FOREACH_FROM
+  - LIST_FOREACH_FROM_SAFE
+  - LIST_FOREACH_SAFE
+  - LIST_FOR_EACH_ENTRY
+  - LIST_FOR_EACH_ENTRY_SAFE
+  - MLX5_ETH_FOREACH_DEV
+  - MLX5_IPOOL_FOREACH
+  - MLX5_L3T_FOREACH
+  - 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
+  - OSAL_LIST_FOR_EACH_ENTRY
+  - OSAL_LIST_FOR_EACH_ENTRY_SAFE
+  - PLT_TAILQ_FOREACH_SAFE
+  - RTE_BBDEV_FOREACH
+  - RTE_BBDEV_FOREACH
+  - RTE_DEV_FOREACH
+  - RTE_DEV_FOREACH
+  - RTE_DMA_FOREACH_DEV
+  - RTE_DMA_FOREACH_DEV
+  - RTE_EAL_DEVARGS_FOREACH
+  - RTE_EAL_DEVARGS_FOREACH
+  - RTE_ETH_FOREACH_DEV
+  - RTE_ETH_FOREACH_DEV
+  - RTE_ETH_FOREACH_DEV_OF
+  - RTE_ETH_FOREACH_DEV_OF
+  - RTE_ETH_FOREACH_DEV_OWNED_BY
+  - RTE_ETH_FOREACH_DEV_OWNED_BY
+  - RTE_ETH_FOREACH_DEV_SIBLING
+  - RTE_ETH_FOREACH_DEV_SIBLING
+  - RTE_ETH_FOREACH_MATCHING_DEV
+  - RTE_ETH_FOREACH_MATCHING_DEV
+  - RTE_ETH_FOREACH_VALID_DEV
+  - RTE_ETH_FOREACH_VALID_DEV
+  - RTE_GPU_FOREACH
+  - RTE_GPU_FOREACH
+  - RTE_GPU_FOREACH_CHILD
+  - RTE_GPU_FOREACH_CHILD
+  - RTE_GPU_FOREACH_PARENT
+  - RTE_GPU_FOREACH_PARENT
+  - RTE_LCORE_FOREACH
+  - RTE_LCORE_FOREACH
+  - RTE_LCORE_FOREACH_WORKER
+  - RTE_LCORE_FOREACH_WORKER
+  - RTE_TAILQ_FOREACH
+  - RTE_TAILQ_FOREACH
+  - RTE_TAILQ_FOREACH_SAFE
+  - 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

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

[Abdullah Ömer Yamaç]

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

On Sun, May 5, 2024 at 11:38 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Sun, 5 May 2024 22:42:57 +0300
> Abdullah Ömer Yamaç <aomeryamac@gmail.com> wrote:
>
> > > Also, this looks wrong.  The initialized arrays looked better before.
> > >
> > >
> > > -static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {
> > > -       "UNKNOWN", "TUN", "TAP"
> > > -};
> > > +static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {"UNKNOWN",
> "TUN",
> > > "TAP"};
> > >
> > > -static const char *valid_arguments[] = {
> > > -       ETH_TAP_IFACE_ARG,
> > > -       ETH_TAP_REMOTE_ARG,
> > > -       ETH_TAP_MAC_ARG,
> > > -       ETH_TAP_PERSIST_ARG,
> > > -       NULL
> > > -};
> > > +static const char *valid_arguments[] = {ETH_TAP_IFACE_ARG,
> > > ETH_TAP_REMOTE_ARG, ETH_TAP_MAC_ARG,
> > > +                                       ETH_TAP_PERSIST_ARG, NULL};
> > >
> >
> > I am confused about these variables.  tuntap_types list values in a
> single
> > line, but valid_arguments' values are listed separately.
> > So, it is getting more complex to handle both of them. What should we do,
> > do you have any idea?
>
> Ignore the initialized lists for now. It should be possible to have it
> generate something
> like
>
> static const char *tuntap_types[ETH_TUNTAP_TYPE_MAX] = {
>       "UNKNOWN", "TUN", "TAP"
> };
>
> With the following changes result looks better. You got the format wrong
> for the ForEach list.
>
> diff --git a/.clang-format b/.clang-format
> index 16164ef1de..d16185c049 100644
> --- a/.clang-format
> +++ b/.clang-format
> @@ -1,12 +1,20 @@
>  ---
>  BasedOnStyle: LLVM
>
> +AttributeMacros:
> +  - __rte_aligned
> +  - __rte_packed
> +  - __rte_may_alias
> +  - __rte_deprecated
> +  - __rte_weak
> +  - __rte_unused
> +  - __rte_restrict
> +
>  # Place opening and closing parentheses on the same line for control
> statements
>  BreakBeforeBraces: Custom
>  BraceWrapping:
> -        AfterFunction: false
> +        AfterFunction: true
>          AfterControlStatement: false
> -        AfterEnum: false
>
> These are ok for me.

>  # Set maximum line length to 100 characters
>  ColumnLimit: 100
> @@ -41,98 +49,117 @@ AlwaysBreakAfterReturnType: TopLevelDefinitions
>  # Always break before multiline string literals
>  AlignEscapedNewlines: Left
>
> -# Foreach macros
>
In the clang documentation, it says "ForEachMacros (List of Strings)" and
gives an example: "ForEachMacros: ['RANGES_FOR', 'FOREACH']"

> ...
> +ObjCSpaceAfterProperty: true
> +IndentGotoLabels: false
>
These are also ok for me.

If you agree on the for-each part, then I will send the new patch.
Thanks

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

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

[Abdullah Ömer Yamaç]

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

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

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

[Ferruh Yigit]

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

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

[Stephen Hemminger]

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.

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

[Morten Brørup]

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

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

[Bruce Richardson]

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

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

[Tyler Retzlaff]

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.

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

[Abdullah Ömer Yamaç]

[-- 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 --]

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

[Bruce Richardson]

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

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

[Abdullah Ömer Yamaç]

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

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
>

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

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

[Bruce Richardson]

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

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

[Stephen Hemminger]

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.

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

[Abdullah Ömer Yamaç]

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

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

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

[PATCH v5] devtools: add .clang-format file

[Abdullah Ömer Yamaç]

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 | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 149 insertions(+)
 create mode 100644 .clang-format

diff --git a/.clang-format b/.clang-format
new file mode 100644
index 0000000000..5f86e1be79
--- /dev/null
+++ b/.clang-format
@@ -0,0 +1,149 @@
+---
+BasedOnStyle: LLVM
+
+# Place opening and closing parentheses on the same line for control statements
+BreakBeforeBraces: Custom
+BraceWrapping:
+        AfterFunction: true
+        AfterControlStatement: false
+
+AllowShortEnumsOnASingleLine: false
+
+# Should be declared this way:
+SpaceBeforeParens: Custom
+SpaceBeforeParensOptions:
+        AfterForeachMacros: 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 16 spaces (2 tabs)
+AlignAfterOpenBracket: DontAlign
+ContinuationIndentWidth: 16
+
+# 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
-- 
2.34.1

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

[Bruce Richardson]

On Thu, May 16, 2024 at 08:20:51AM +0000, 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>
> ---
>  .clang-format | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 149 insertions(+)
>  create mode 100644 .clang-format
> 
I think this is useful to have for new code esp. new libs/drivers.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

[Abdullah Ömer Yamaç]

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

> ...
>
> 1. Some options are failing for me [1], I don't know if it requires a
> specific version of clang-format
>
I fixed these errors, and the clang-format version should be 17. I will
send them after some discussions, as I've shared below.

>
> 2. Current options are not fully aligned with coding convention, it is
> easier to see what is not compatible by running the tool on an existing
> file [2].
> we need to fix them.
>
> There are some cases that we need to discuss. First of all, I applied the
latest clang format that I created.
- In default cases for clang-format, include headers are sorted
alphabetically. It improves the readability of code, and it breaks the
codebase. These changes do not affect code maintenance because they are
only headers.
- Second question about the column width. In the definition of
.editorconfig, the column width is set as 100, but the previous commits
violate this. Here is the sample code:
 struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
@@ -61,8 +61,7 @@ static const struct rte_eth_xstats_name_off
eth_dev_stats_strings[] = {
        {"rx_missed_errors", offsetof(struct rte_eth_stats, imissed)},
        {"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
        {"tx_errors", offsetof(struct rte_eth_stats, oerrors)},
-       {"rx_mbuf_allocation_errors", offsetof(struct rte_eth_stats,
-               rx_nombuf)},
+       {"rx_mbuf_allocation_errors", offsetof(struct rte_eth_stats,
rx_nombuf)},
 };

We could use this clang-format file only for the new patches and newly
inserted lines. I am using VSCode IDE for development, and after some
modification, I can select the modified lines and apply formatting. It is
very useful for these cases. Otherwise, we need to handle so many things.

By the way, I am still working on macros. If you have any comments on this,
I would be very happy to discuss it.


> ...

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