|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 against the coding standards and contribution guidelines.

---

commit 14bf61a7c5e2f8049d4c8e5a9f5c6d3e2a8b9c4d
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:27 -0700

examples/ethtool: fix shadow variable warning

Fix shadow variable warning by renaming the function parameter from
'app_cfg' to 'ptr_cfg' to avoid shadowing the global variable.

../examples/ethtool/ethtool-app/main.c:89:44: warning: declaration of
'app_cfg' shadows a global declaration [-Wshadow]

Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>

> --- 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 renaming to 'ptr_cfg' really improve clarity when there's already a
global app_cfg? Would using a name that describes the actual purpose be
clearer, like 'config' or 'port_config'? The 'ptr_' prefix doesn't add
semantic value.

>  {
>  	int idx_port;
>  	int size_pktpool;
> @@ -101,7 +101,7 @@ static void setup_ports(struct app_config *app_cfg, int cnt_ports)
>  	cfg_port.txmode.mq_mode = RTE_ETH_MQ_TX_NONE;
>  
>  	for (idx_port = 0; idx_port < cnt_ports; idx_port++) {
> -		struct app_port *ptr_port = &app_cfg->ports[idx_port];
> +		struct app_port *ptr_port = &ptr_cfg->ports[idx_port];

[ ... ]

---

commit 2e8a3b4c7d9f1e0a5b6c8d2f3a4e5b6c7d8e9f0a
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:28 -0700

examples/eventdev_pipeline: fix shadow variable warning

Fix shadow variable warning by reusing the existing 'ret' variable
instead of declaring a new one with the same name.

> --- a/examples/eventdev_pipeline/pipeline_worker_tx.c
> +++ b/examples/eventdev_pipeline/pipeline_worker_tx.c
> @@ -874,8 +874,8 @@ init_adapters(uint16_t nb_ports)
>  		service.callback = service_rx_adapter;
>  		service.callback_userdata = (void *)adptr_services;
>  
> -		int32_t ret = rte_service_component_register(&service,
> -				&fdata->rxadptr_service_id);
> +		ret = rte_service_component_register(&service,
> +					&fdata->rxadptr_service_id);

Does this code properly check ret for errors from the first usage before
this point? If ret is used for a different purpose earlier in the
function, reusing it here could mask errors.

>  		if (ret)
>  			rte_exit(EXIT_FAILURE,
>  				"Rx adapter service register failed");

---

commit 3f7c5a9d2b8e1f4c6a7d9b3e0f2a5c6d8e1f9b4a
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:29 -0700

examples/dma: fix shadow variable warnings

> --- a/examples/dma/dmafwd.c
> +++ b/examples/dma/dmafwd.c
> @@ -590,11 +590,11 @@ dma_parse_portmask(const char *portmask)
>  }
>  
>  static copy_mode_t
> -dma_parse_copy_mode(const char *copy_mode)
> +dma_parse_copy_mode(const char *mode)
>  {
> -	if (strcmp(copy_mode, COPY_MODE_SW) == 0)
> +	if (strcmp(mode, COPY_MODE_SW) == 0)
>  		return COPY_MODE_SW_NUM;
> -	else if (strcmp(copy_mode, COPY_MODE_DMA) == 0)
> +	else if (strcmp(mode, COPY_MODE_DMA) == 0)
>  		return COPY_MODE_DMA_NUM;
>  
>  	return COPY_MODE_INVALID_NUM;

[ ... ]

> @@ -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 this code call port_init() only with the global nb_queues value? If
the function relies on a global instead of a parameter, does it reduce
testability or create implicit coupling?

---

commit 4a8c6d5e7f9b2a1c3d4e6f8a9b0c1d2e3f4a5b6c
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:31 -0700

examples/bond: fix shadow variable warnings

> --- a/examples/bond/main.c
> +++ b/examples/bond/main.c
> @@ -96,13 +96,12 @@
>  		RTE_ETHER_ADDR_BYTES(&addr))
>  
> -uint16_t members[RTE_MAX_ETHPORTS];
> -uint16_t members_count;
> +static uint16_t members[RTE_MAX_ETHPORTS];
> +static uint16_t members_count;

Does making these variables static to the file change the intended
linkage or external visibility? Were these previously used from other
translation units?

[ ... ]

---

commit 7c9d2e4f3a8b1c5d6e9f0a2b3c4d5e6f7a8b9c0d
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:40 -0700

examples/ptpclient: fix shadow variable warnings

> --- a/examples/ptpclient/ptpclient.c
> +++ b/examples/ptpclient/ptpclient.c
> @@ -173,7 +173,7 @@ ns_to_timeval(int64_t nsec)
>   * coming from the mbuf_pool passed as a parameter.
>   */
>  static inline int
> -port_init(uint16_t port, struct rte_mempool *mbuf_pool)
> +port_init(uint16_t port, struct rte_mempool *mp)

Would 'pool' or 'mempool' be more readable than 'mp'? The abbreviation
'mp' could be confused with 'multiprocess'.

[ ... ]

---

commit 8e1f0b9a2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:48 -0700

examples/l3fwd: fix shadow variable warnings

> --- a/examples/l3fwd/l3fwd.h
> +++ b/examples/l3fwd/l3fwd.h
> @@ -99,7 +99,7 @@ extern struct rte_ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
>  extern uint32_t enabled_port_mask;
>  
>  /* Used only in exact match mode. */
> -extern int ipv6; /**< ipv6 is false by default. */
> +extern bool ipv6_enabled; /**< ipv6 is false by default. */

Does changing from 'int ipv6' to 'bool ipv6_enabled' affect ABI if this
is in a public header? Are there any existing users that might be broken
by this type change?

[ ... ]

---

commit 9f2a3b4c5d6e7f8a9b0c1d2e3f4a5b6c7d8e9f0a
Author: Stephen Hemminger <stephen at networkplumber.org>
Date: Mon, 6 Apr 2026 20:49:42 -0700

examples/qos_sched: eliminate shadowed variables

> --- a/examples/qos_sched/cfg_file.c
> +++ b/examples/qos_sched/cfg_file.c
> @@ -41,20 +41,20 @@ int parse_u64(const char *entry, uint64_t *val)
>  }
>  
>  int
> -cfg_load_port(struct rte_cfgfile *cfg, struct rte_sched_port_params *port_params)
> +cfg_load_port(struct rte_cfgfile *cfg)
>  {
>  	const char *entry;
>  
> -	if (!cfg || !port_params)
> +	if (!cfg)
>  		return -1;

Does cfg_load_port() modify a global port_params? If so, does this reduce
the function's reusability or make it harder to test in isolation?

[ ... ]

---