Re: [PATCH v4 00/31] build DPDK with Wshadow flag

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 14 Jan 2026 15:44:14 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> As flagged in bugs #1742, #1743 and others, much of DPDK fails to build
> when -Wshadow flag is passed to the build. This patchset fixes many issues
> of this nature, but not all, in the hopes we can move towards enabling that
> warning flag for all components for DPDK in future. This patchset fixes the
> following components in DPDK:
> 
> * all libraries
> * Intel net drivers
> * all apps in the "app" folder
> 
> For the rest of the drivers and example applications, any component which
> does not build successfully with the shadow warning enabled has it
> explicitly disabled via cflags. Then the last patch adds -Wshadow to the
> global DPDK build, which should prevent any regressions in components which
> are already working.
> 
> At that point, we should not have any regressions and we can, over time,
> reduce the number of components needing the -Wno-shadow flag.
> 
> V4:
> * Updates follow AI review:
>   - improve variable naming in min/max fns
>   - remove extra blank line
> * rebase on latest main
> 
> v3:
> * rebase on 25.11 release.
> 
> RFC v2:
> * take patch from Stephen for RTE_MIN/MAX fixes
> * add additional fixes to clear clang warnings - it's a lot stricter than
>   gcc in this regard, and flags more issues.
> * extend app cleanup to all apps, not just testpmd
> * add per-component disabling, and add global enable flag at the end.
> 
> Bruce Richardson (30):
>   eal: fix variable shadowing
>   ethdev: fix variable shadowing issues
>   eventdev: fix variable shadowing issues
>   net: remove shadowed variable
>   pipeline: fix variable shadowing
>   table: fix issues with variable shadowing
>   power: rename variable to eliminate shadowing
>   pcapng: rename variable to fix shadowing
>   bbdev: fix variable shadowing
>   bus/pci: remove shadowed variables
>   net/intel: rename function param to avoid shadow warnings
>   net/e1000: fix build with shadow warnings enabled
>   net/i40e: fix build with shadow warnings enabled
>   net/ice: fix build with shadow warnings enabled
>   net/cpfl: fix build with shadow warnings enabled
>   net/ixgbe: fix build with shadow warnings enabled
>   app/testpmd: fix build with shadow warnings enabled
>   app/graph: fix build with shadow warnings enabled
>   app/pdump: fix warning about shadowed variable
>   app/test-bbdev: remove shadow warning from next max calls
>   app/test-compress-perf: rename local vars to fix shadowing
>   app/test-crypto-perf: rename local vars to fix shadowing
>   app/test-eventdev: fix build with shadow warnings enabled
>   app/test-flow-perf: remove unneeded variable
>   app/test-security-perf: fix build with shadow warnings
>   app/test-pipeline: remove unnecessary variable
>   drivers: disable variable shadowing warnings for drivers
>   app/test: disable shadowing warnings for unit tests
>   examples: ignore variable shadowing warnings
>   build: enable shadowed variable warnings
> 
> Stephen Hemminger (1):
>   eal: add more min/max helpers
> 
>  app/graph/conn.c                              | 134 +++++++++---------
>  app/pdump/main.c                              |  12 +-
>  app/test-bbdev/test_bbdev_perf.c              |   7 +-
>  app/test-compress-perf/main.c                 | 122 ++++++++--------
>  .../cperf_test_pmd_cyclecount.c               |   6 +-
>  app/test-eventdev/evt_main.c                  |   6 +-
>  app/test-eventdev/test_atomic_queue.c         |   6 +-
>  app/test-eventdev/test_perf_common.c          |   4 +-
>  app/test-flow-perf/main.c                     |   1 -
>  app/test-pipeline/main.c                      |   1 -
>  app/test-pmd/cmdline_flow.c                   |  42 +++---
>  app/test-pmd/config.c                         |  15 +-
>  app/test-pmd/parameters.c                     |  38 +++--
>  app/test-pmd/testpmd.c                        |   2 +-
>  app/test-security-perf/test_security_perf.c   |  42 +++---
>  app/test/meson.build                          |   1 +
>  app/test/test_cryptodev_security_ipsec.c      |  13 +-
>  config/meson.build                            |   6 +
>  drivers/baseband/fpga_5gnr_fec/meson.build    |   2 +
>  drivers/bus/fslmc/meson.build                 |   2 +
>  drivers/bus/pci/linux/pci.c                   |   1 -
>  drivers/bus/pci/linux/pci_vfio.c              |   2 +-
>  drivers/bus/pci/windows/pci.c                 |   1 -
>  drivers/common/cnxk/meson.build               |   1 +
>  drivers/common/qat/meson.build                |   1 +
>  drivers/compress/nitrox/meson.build           |   2 +
>  drivers/crypto/cnxk/meson.build               |   1 +
>  drivers/crypto/octeontx/meson.build           |   1 +
>  drivers/crypto/openssl/meson.build            |   1 +
>  drivers/crypto/scheduler/meson.build          |   1 +
>  drivers/dma/dpaa2/meson.build                 |   2 +
>  drivers/event/cnxk/meson.build                |   1 +
>  drivers/event/dlb2/meson.build                |   2 +
>  drivers/event/sw/meson.build                  |   1 +
>  drivers/net/axgbe/meson.build                 |   2 +-
>  drivers/net/bnxt/meson.build                  |   1 +
>  drivers/net/bonding/meson.build               |   1 +
>  drivers/net/cnxk/meson.build                  |   1 +
>  drivers/net/cxgbe/meson.build                 |   1 +
>  drivers/net/dpaa/meson.build                  |   1 +
>  drivers/net/dpaa2/meson.build                 |   1 +
>  drivers/net/ena/meson.build                   |   2 +
>  drivers/net/enetfec/meson.build               |   2 +
>  drivers/net/enic/meson.build                  |   2 +
>  drivers/net/failsafe/meson.build              |   1 +
>  drivers/net/intel/common/rx.h                 |   6 +-
>  drivers/net/intel/cpfl/cpfl_flow_engine_fxp.c |   2 -
>  drivers/net/intel/cpfl/cpfl_flow_parser.c     |   6 +-
>  drivers/net/intel/cpfl/cpfl_fxp_rule.h        |   2 -
>  drivers/net/intel/cpfl/cpfl_representor.h     |   2 +-
>  drivers/net/intel/e1000/igc_ethdev.c          |   6 +-
>  drivers/net/intel/i40e/i40e_ethdev.h          |  10 +-
>  drivers/net/intel/ice/ice_ethdev.c            |   6 +-
>  drivers/net/intel/ixgbe/ixgbe_fdir.c          |   7 +-
>  drivers/net/mlx5/meson.build                  |   1 +
>  drivers/net/ntnic/meson.build                 |   3 +
>  drivers/net/pfe/meson.build                   |   3 +
>  drivers/net/qede/meson.build                  |   3 +
>  drivers/net/txgbe/meson.build                 |   1 +
>  drivers/net/zxdh/meson.build                  |   3 +
>  drivers/raw/ifpga/meson.build                 |   2 +
>  drivers/vdpa/mlx5/meson.build                 |   1 +
>  examples/bbdev_app/meson.build                |   1 +
>  examples/bond/meson.build                     |   1 +
>  examples/dma/meson.build                      |   1 +
>  examples/ethtool/meson.build                  |   1 +
>  examples/eventdev_pipeline/meson.build        |   1 +
>  examples/flow_filtering/meson.build           |   1 +
>  examples/ip_pipeline/meson.build              |   1 +
>  examples/ipsec-secgw/meson.build              |   1 +
>  examples/l2fwd-crypto/meson.build             |   1 +
>  examples/l2fwd-event/meson.build              |   1 +
>  examples/l2fwd-jobstats/meson.build           |   1 +
>  examples/l2fwd-keepalive/meson.build          |   1 +
>  examples/l3fwd-graph/meson.build              |   1 +
>  examples/l3fwd-power/meson.build              |   1 +
>  examples/l3fwd/meson.build                    |   1 +
>  .../client_server_mp/mp_server/meson.build    |   1 +
>  examples/packet_ordering/meson.build          |   1 +
>  examples/ptpclient/meson.build                |   1 +
>  examples/qos_sched/meson.build                |   1 +
>  .../server_node_efd/efd_server/meson.build    |   1 +
>  examples/vhost/meson.build                    |   1 +
>  .../vm_power_manager/guest_cli/meson.build    |   1 +
>  examples/vm_power_manager/meson.build         |   1 +
>  examples/vmdq/meson.build                     |   1 +
>  lib/bbdev/rte_bbdev.c                         |   2 +-
>  lib/eal/common/eal_common_options.c           |  22 +--
>  lib/eal/common/eal_common_trace.c             |  89 ++++++------
>  lib/eal/common/malloc_heap.c                  |   2 +-
>  lib/eal/include/rte_common.h                  |  38 ++++-
>  lib/ethdev/ethdev_driver.c                    |   6 +-
>  lib/ethdev/rte_ethdev.c                       |   1 -
>  lib/eventdev/rte_event_eth_rx_adapter.c       |   5 +-
>  lib/eventdev/rte_event_eth_tx_adapter.c       |   8 +-
>  lib/net/net_crc_avx512.c                      |   4 +-
>  lib/pcapng/rte_pcapng.c                       |   7 +-
>  lib/pipeline/rte_swx_ctl.c                    |  11 +-
>  lib/pipeline/rte_swx_pipeline.c               |  21 ++-
>  lib/pipeline/rte_swx_pipeline_internal.h      |  10 +-
>  lib/power/rte_power_pmd_mgmt.c                |   6 +-
>  lib/table/rte_lru.h                           |  38 ++---
>  lib/table/rte_lru_arm64.h                     |  14 +-
>  lib/table/rte_lru_x86.h                       |   4 +-
>  lib/table/rte_swx_table_em.c                  |   2 -
>  lib/table/rte_swx_table_learner.c             |   4 +-
>  lib/table/rte_table_hash_key16.c              |   4 +-
>  lib/table/rte_table_hash_key32.c              |   4 +-
>  lib/table/rte_table_hash_key8.c               |   4 +-
>  109 files changed, 477 insertions(+), 412 deletions(-)
> 
> --
> 2.51.0
> 

AI review gives thumbs up on this...



## DPDK Patch Series Review: Shadow Warning Fixes (v4)

### Series Overview

| Aspect | Details |
|--------|---------|
| **Series** | [PATCH v4 01-31/31] Shadow warning fixes |
| **Submitter** | Bruce Richardson (Intel) |
| **Original Author (Patch 1)** | Stephen Hemminger |
| **Purpose** | Enable `-Wshadow` compiler warning globally |

---

## Patch-by-Patch Evaluation

### Patch 01/31: eal: add more min/max helpers

**Commit Message:**
| Check | Status | Notes |
|-------|--------|-------|
| Subject ≤60 chars | ✓ | "eal: add more min/max helpers" (27 chars) |
| Correct prefix | ✓ | `eal:` is correct for lib/eal |
| Lowercase after colon | ✓ | |
| Imperative mood | ✓ | "add" |
| No trailing period | ✓ | |
| Signed-off-by present | ✓ | Stephen Hemminger + Bruce Richardson |
| Body does not start with "It" | ✓ | |

**Code Review:**
- **Logic**: Adds `RTE_MIN3()` and `RTE_MAX3()` macros, and renames internal variables (`_a`→`_a_min`, etc.) to avoid shadow conflicts when nesting MIN/MAX calls. Sound approach.

**Warning - Minor style issue in RTE_MIN3:**
```c
		_a_min3 < _b_min3 ? \
			(_a_min3 < _c_min3 ? _a_min3 : _c_min3) : \
		        (_b_min3 < _c_min3 ? _b_min3 : _c_min3);  \
```
Line 3 has inconsistent indentation (spaces vs tabs for alignment) and a trailing double-space before the backslash. The same issue exists in `RTE_MAX3`.

---

### Patch 02/31: eal: fix variable shadowing

**Commit Message:**
| Check | Status | Notes |
|-------|--------|-------|
| Subject ≤60 chars | ✓ | 26 chars |
| Bugzilla ID present | ✓ | IDs 1742 and 1743 |
| Fixes: tags | ✓ | 3 Fixes with 12-char SHA and exact subjects |
| Cc: stable@dpdk.org | ✓ | Present for bug fix |
| Tag order | ✓ | Bugzilla ID → Fixes → Cc → Signed-off-by → Acked-by |

**Code Review:** Clean refactoring - renames `trace` to `t` for local variables, renames `args` to `out_args`, removes redundant `aligned_end` redefinition.

---

### Patches 03-11: Library Fixes

All follow the same pattern for ethdev, eventdev, net, pipeline, table, power, pcapng, bbdev, and bus/pci. Each:
- Has proper `Fixes:` tags with 12-char SHA
- Has `Cc: stable@dpdk.org`  
- Has appropriate Acked-by tags
- Correctly removes shadowed variables by renaming or removing redundant definitions

---

### Patches 12-17: Intel Driver Fixes (net/intel, e1000, i40e, ice, cpfl, ixgbe)

**Patch 14 (i40e) - Good macro fix:**
```c
-		struct rte_eth_dev *dev;\
+		struct rte_eth_dev *_dev;\
```
Correctly prefixes macro-internal variables with `_` to avoid shadowing.

**Patch 15 (ice):** Converts nested `RTE_MIN(RTE_MIN(...))` to two separate calls - necessary workaround for the old macro implementation.

---

### Patches 18-27: Application Fixes (testpmd, app/graph, pdump, etc.)

**Patch 18 (testpmd):** Several good fixes:
- Renames `opt` to `geneve_opt`, `gtp_opt`, `gre_opt` for clarity
- Removes unnecessary `rss_hf` intermediate variable
- Renames `optarg` parameter to `event_arg` to avoid shadowing the global `optarg` from getopt

**Patch 19 (app/graph):** Renames function parameter `conn` to `c` throughout to avoid shadowing global variable. Good systematic approach.

---

### Patches 28-30: Bulk Disable for Drivers/Tests/Examples

These patches add `cflags += no_shadow_cflag` to meson.build files for components that have extensive shadow warnings. This is a pragmatic approach for:
- **Drivers**: ~35 drivers that need cleanup later
- **app/test**: Unit tests with many warnings
- **examples**: ~24 example apps

---

### Patch 31/31: Enable -Wshadow globally

Adds `-Wshadow` to the global compiler flags, completing the series.

---

## Overall Assessment

### Errors (Must Fix)
**None found.** All commit messages follow DPDK standards.

### Warnings (Should Fix)

| Issue | Location | Severity |
|-------|----------|----------|
| Inconsistent indentation (spaces vs tabs) | Patch 01, RTE_MIN3/RTE_MAX3 | Warning |
| Trailing double-space before backslash | Patch 01, line endings | Warning |

### Info (Consider)

1. **Patch 01**: The new `RTE_MIN3`/`RTE_MAX3` macros lack `@param` documentation in the Doxygen comment. Consider adding:
   ```c
   /**
    * Macro to return the minimum of three numbers
    * @param a First number
    * @param b Second number  
    * @param c Third number
    */
   ```

2. **Series organization**: The series is well-structured, fixing core issues first, then building up through libraries, drivers, and apps before enabling the warning globally.

---

## Verdict

**Recommended: Acked-by** with minor style fixes requested for Patch 01.

The series is comprehensive and well-executed. It systematically enables an important compiler warning while maintaining buildability by temporarily disabling it where extensive cleanup would be needed. The approach of fixing core libraries first and deferring driver/example cleanup is pragmatic for a large codebase.