* [net-next] net: ravb: Fix -Wmaybe-uninitialized warning
@ 2025-09-12 13:06 Niklas Söderlund
2025-09-12 14:06 ` Przemek Kitszel
0 siblings, 1 reply; 2+ messages in thread
From: Niklas Söderlund @ 2025-09-12 13:06 UTC (permalink / raw)
To: Paul Barker, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-renesas-soc
Cc: Niklas Söderlund
Fix a -Wmaybe-uninitialized warning by initializing the variable to
NULL. The warning is bogus and should not happen, but fixing it allows
running the check on the driver to catch potential future problems.
$ make CFLAGS_ravb_main.o=-Wmaybe-uninitialized
In function 'ravb_rx_csum_gbeth',
inlined from 'ravb_rx_gbeth' at .../linux/drivers/net/ethernet/renesas/ravb_main.c:923:6:
.../linux/drivers/net/ethernet/renesas/ravb_main.c:765:25: error: 'skb' may be used uninitialized [-Werror=maybe-uninitialized]
765 | if (unlikely(skb->len < csum_len))
| ~~~^~~~~
.../linux/include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
77 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
.../linux/drivers/net/ethernet/renesas/ravb_main.c: In function 'ravb_rx_gbeth':
.../linux/drivers/net/ethernet/renesas/ravb_main.c:806:25: note: 'skb' was declared here
806 | struct sk_buff *skb;
| ^~~
cc1: all warnings being treated as errors
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 94b6fb94f8f1..9d3bd65b85ff 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -802,7 +802,6 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
const struct ravb_hw_info *info = priv->info;
struct net_device_stats *stats;
struct ravb_rx_desc *desc;
- struct sk_buff *skb;
int rx_packets = 0;
u8 desc_status;
u16 desc_len;
@@ -815,6 +814,8 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
stats = &priv->stats[q];
for (i = 0; i < limit; i++, priv->cur_rx[q]++) {
+ struct sk_buff *skb = NULL;
+
entry = priv->cur_rx[q] % priv->num_rx_ring[q];
desc = &priv->rx_ring[q].desc[entry];
if (rx_packets == budget || desc->die_dt == DT_FEMPTY)
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [net-next] net: ravb: Fix -Wmaybe-uninitialized warning
2025-09-12 13:06 [net-next] net: ravb: Fix -Wmaybe-uninitialized warning Niklas Söderlund
@ 2025-09-12 14:06 ` Przemek Kitszel
0 siblings, 0 replies; 2+ messages in thread
From: Przemek Kitszel @ 2025-09-12 14:06 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Paul Barker, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-renesas-soc
On 9/12/25 15:06, Niklas Söderlund wrote:
> Fix a -Wmaybe-uninitialized warning by initializing the variable to
> NULL. The warning is bogus and should not happen, but fixing it allows
> running the check on the driver to catch potential future problems.
Code is indeed a bit complicated state machine, now wonders static
analysis was fooled
>
> $ make CFLAGS_ravb_main.o=-Wmaybe-uninitialized
>
> In function 'ravb_rx_csum_gbeth',
> inlined from 'ravb_rx_gbeth' at .../linux/drivers/net/ethernet/renesas/ravb_main.c:923:6:
I see that we reference skb before calling the inlined function, so the
problem (false or not) should really be reported there
> .../linux/drivers/net/ethernet/renesas/ravb_main.c:765:25: error: 'skb' may be used uninitialized [-Werror=maybe-uninitialized]
> 765 | if (unlikely(skb->len < csum_len))
> | ~~~^~~~~
> .../linux/include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
> 77 | # define unlikely(x) __builtin_expect(!!(x), 0)
> | ^
> .../linux/drivers/net/ethernet/renesas/ravb_main.c: In function 'ravb_rx_gbeth':
> .../linux/drivers/net/ethernet/renesas/ravb_main.c:806:25: note: 'skb' was declared here
> 806 | struct sk_buff *skb;
> | ^~~
> cc1: all warnings being treated as errors
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
would be great to supply what was the compiler (incl version)
> ---
> drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 94b6fb94f8f1..9d3bd65b85ff 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -802,7 +802,6 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
> const struct ravb_hw_info *info = priv->info;
> struct net_device_stats *stats;
> struct ravb_rx_desc *desc;
> - struct sk_buff *skb;
> int rx_packets = 0;
> u8 desc_status;
> u16 desc_len;
> @@ -815,6 +814,8 @@ static int ravb_rx_gbeth(struct net_device *ndev, int budget, int q)
> stats = &priv->stats[q];
>
> for (i = 0; i < limit; i++, priv->cur_rx[q]++) {
> + struct sk_buff *skb = NULL;
this does not break the current correctness, (no matter what is the
order of incoming die_dt's (even if broken/"impossible"), and makes it
a little bit easier for the future analysis, thanks to smaller var scope
I have no idea why "maybe null" is better for static analysis than
"maybe uninitialized", but, as this decreases scope of the variable,
this change could be marked as good anyway, so feel free to add my tag:
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
perhaps compiler could not assume that priv->rx_1st_skb was 0 initially
(assumed it could be a non-zero garbage)
> +
> entry = priv->cur_rx[q] % priv->num_rx_ring[q];
> desc = &priv->rx_ring[q].desc[entry];
> if (rx_packets == budget || desc->die_dt == DT_FEMPTY)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-09-12 14:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-12 13:06 [net-next] net: ravb: Fix -Wmaybe-uninitialized warning Niklas Söderlund
2025-09-12 14:06 ` Przemek Kitszel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).