|SUCCESS| [23/23] examples/l3fwd-power: fix shadow variable warnings

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162884

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-07

I'll review this DPDK patch series that fixes shadow variable warnings across multiple examples.

git sha: (multiple patches in series)
Author: Stephen Hemminger <stephen at networkplumber.org>
Subject: [PATCH 01-23/23] examples: fix shadow variable warnings

This patch series removes shadow variable warnings from various DPDK examples by renaming conflicting variables.

> diff --git a/examples/ethtool/ethtool-app/main.c b/examples/ethtool/ethtool-app/main.c
> index b6bbae70d2..6545eb322d 100644
> --- a/examples/ethtool/ethtool-app/main.c
> +++ b/examples/ethtool/ethtool-app/main.c
> @@ -86,7 +86,7 @@ void mark_port_newmac(int idx_port)
>  	ptr_port->port_dirty = 1;
>  }
>  
> -static void setup_ports(struct app_config *app_cfg, int cnt_ports)
> +static void setup_ports(struct app_config *ptr_cfg, int cnt_ports)

Does the function actually use the global app_cfg anywhere? If setup_ports()
only uses its parameter, this rename is correct.

> diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
> index 5ba0aaa40b..a8ed09e6ae 100644
> --- a/examples/dma/dmafwd.c
> +++ b/examples/dma/dmafwd.c
> @@ -878,7 +878,7 @@ config_port_max_pkt_len(struct rte_eth_conf *conf,
>   * coming from the mbuf_pool passed as a parameter.
>   */
>  static inline void
> -port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
> +port_init(uint16_t portid, struct rte_mempool *mbuf_pool)

Does the code still compile when nb_queues parameter is removed? Can you verify
port_init() correctly accesses the global nb_queues throughout the function?

> diff --git a/examples/bond/main.c b/examples/bond/main.c
> index 4e8eeb7a5e..99359cd6fb 100644
> --- a/examples/bond/main.c
> +++ b/examples/bond/main.c
> @@ -635,7 +634,7 @@ cmd_quit_parsed(__rte_unused void *parsed_result, struct cmdline *cl, __rte_unus
>  void
>  cmd_show_parsed(__rte_unused void *parsed_result, struct cmdline *cl, __rte_unused void *data)
>  {
> -	uint16_t members[16] = {0};
> +	uint16_t act_members[16] = {0};

Does this code correctly distinguish between the global members[] array and
the local act_members[] array? The rename is good, but verify the logic
doesn't get confused.

> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index a1a996dee8..866ba04b86 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1855,9 +1854,9 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>  				rule_type ==
>  				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>  				&& rule->portid == port_id)
> -			*rx_offloads |= RTE_ETH_RX_OFFLOAD_SECURITY;
> +			*rx_flags |= RTE_ETH_RX_OFFLOAD_SECURITY;

Does the function signature change from rx_offloads to rx_flags match all
call sites? This is a good semantic improvement.

> diff --git a/examples/ptpclient/ptpclient.c b/examples/ptpclient/ptpclient.c
> index 174ca5dd70..8e1560a3ac 100644
> --- a/examples/ptpclient/ptpclient.c
> +++ b/examples/ptpclient/ptpclient.c
> @@ -530,9 +530,9 @@ update_kernel_time(void)
>   * Parse the DELAY_RESP message.
>   */
>  static void
> -parse_drsp(struct ptpv2_time_receiver_ordinary *ptp_data)
> +parse_drsp(struct ptpv2_time_receiver_ordinary *ptp)

Does shortening ptp_data to ptp introduce any conflicts with other variables
in scope? The rename reduces verbosity which is good.

> diff --git a/examples/l3fwd/l3fwd_em.c b/examples/l3fwd/l3fwd_em.c
> index 58c54ed77e..d8748a0edd 100644
> --- a/examples/l3fwd/l3fwd_em.c
> +++ b/examples/l3fwd/l3fwd_em.c
> @@ -211,9 +211,9 @@ ipv6_hash_crc(const void *data, __rte_unused uint32_t data_len,
>  static alignas(RTE_CACHE_LINE_SIZE) uint8_t ipv4_l3fwd_out_if[L3FWD_HASH_ENTRIES];
>  static alignas(RTE_CACHE_LINE_SIZE) uint8_t ipv6_l3fwd_out_if[L3FWD_HASH_ENTRIES];
>  
> -static rte_xmm_t mask0;
> -static rte_xmm_t mask1;
> -static rte_xmm_t mask2;
> +static rte_xmm_t xmm_mask0;
> +static rte_xmm_t xmm_mask1;
> +static rte_xmm_t xmm_mask2;

Does the xmm_ prefix improve readability here? Are these masks used
consistently throughout the file with the new naming?

> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 4c64194794..8e65472d3d 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -94,7 +94,7 @@ xmm_t val_eth[RTE_MAX_ETHPORTS];
>  uint32_t enabled_port_mask;
>  
>  /* Used only in exact match mode. */
> -int ipv6; /**< ipv6 is false by default. */
> +bool ipv6_enabled; /**< ipv6 is false by default. */

Does changing from int to bool affect any comparisons? This is good
modernization, but verify all usages are compatible with bool type.

The series appears technically sound overall. The changes eliminate genuine
shadow warnings by renaming parameters and local variables that conflicted
with globals. However, please verify:

1. All call sites are updated for function signature changes
2. No semantic changes are introduced accidentally
3. The new names don't introduce new conflicts