|SUCCESS| net/tap: fix close for partially configured port

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-03-26

git sha: 23e2387b49a1 (referenced in Fixes tag)
Author: David Marchand <david.marchand at redhat.com>
Subject: [PATCH] net/tap: fix close for partially configured port

This patch fixes a crash in tap_dev_close() when the device is closed
without having its queues properly configured, by adding NULL checks for
the rx_queues and tx_queues arrays before dereferencing them.

> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 64b359914b..6d710131e8 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1108,17 +1108,22 @@ tap_dev_close(struct rte_eth_dev *dev)
>  #endif
>  
>  	for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
> -		struct rx_queue *rxq = dev->data->rx_queues[i];
> +		struct rx_queue *rxq = NULL;
> +		struct rx_queue *txq = NULL;

Does this introduce a naming error? The variable is declared as "struct
rx_queue *txq" but should this be "struct tx_queue *txq"? If there is no
tx_queue structure and both types of queues share the same structure
definition, this could be confusing for maintainability.

>  
>  		tap_queue_close(process_private, i);
>  
> +		if (dev->data->rx_queues != NULL)
> +			rxq = dev->data->rx_queues[i];

Does this code assume that if rx_queues is non-NULL, then all elements up
to RTE_PMD_TAP_MAX_QUEUES are valid? What happens if rx_queues is
allocated but rx_queues[i] itself is NULL for some indices? The original
code handled the rxq != NULL check after dereferencing, which would work
if the array existed. Should there be a check that rxq is valid after
assignment?

>  		if (rxq != NULL) {
>  			tap_rxq_pool_free(rxq->pool);
>  			rte_free(rxq);
>  			dev->data->rx_queues[i] = NULL;
>  		}
>  
> -		if (dev->data->tx_queues[i] != NULL) {
> +		if (dev->data->tx_queues != NULL)
> +			txq = dev->data->tx_queues[i];
> +		if (txq != NULL) {
>  			rte_free(dev->data->tx_queues[i]);

Is there potential inconsistency here? The code uses the local variable
txq for the NULL check, but then directly accesses
dev->data->tx_queues[i] in the rte_free() call. Should this be
rte_free(txq) instead to ensure the same pointer is freed that was
checked?

>  			dev->data->tx_queues[i] = NULL;
>  		}