devtools: add .clang-format file

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

Checks

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

Commit Message

Abdullah Ömer Yamaç April 29, 2024, 1:04 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 | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 .clang-format
  

Comments

Ferruh Yigit April 29, 2024, 1:32 p.m. UTC | #1
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)
 +       {
```
  
Stephen Hemminger April 29, 2024, 3:36 p.m. UTC | #2
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'
  
Stephen Hemminger April 29, 2024, 3:43 p.m. UTC | #3
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'
  
Abdullah Ömer Yamaç April 30, 2024, 9:27 p.m. UTC | #4
> ...
>
> 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.


> ...
  
Abdullah Ömer Yamaç May 4, 2024, 1:38 p.m. UTC | #5
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'
>
>
>
  

Patch

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