[PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 12 Jul 2026 20:25:10 +0200

The address of a data structure member was determined before
a corresponding null pointer check in the implementation of
the function “rnpgbe_rm_adapter”.

Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
Pass the required address directly to a function call.

This issue was detected by using the Coccinelle software.

Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
Cc: stable@vger.kernel.org
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
index 70a2b0082ba8..93cf757c951c 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_main.c
@@ -245,7 +245,6 @@ static int rnpgbe_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 {
 	struct mucse *mucse = pci_get_drvdata(pdev);
-	struct mucse_hw *hw = &mucse->hw;
 	struct net_device *netdev;
 	int err;
 
@@ -253,7 +252,7 @@ static void rnpgbe_rm_adapter(struct pci_dev *pdev)
 		return;
 	netdev = mucse->netdev;
 	unregister_netdev(netdev);
-	err = rnpgbe_send_notify(hw, false, mucse_fw_powerup);
+	err = rnpgbe_send_notify(&mucse->hw, false, mucse_fw_powerup);
 	if (err)
 		dev_warn(&pdev->dev, "Send powerdown to hw failed %d\n", err);
 	free_netdev(netdev);
-- 
2.55.0

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Vadim Fedorenko]

On 12/07/2026 19:35, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.

The null check in rnpgbe_rm_adapter() looks more like defensive
programming, which got into the code unnoticed. pdev will not exist if
netdev is not allocated, where mucse is netdev private data.

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
> 
> The null check in rnpgbe_rm_adapter() looks more like defensive
> programming,

This can be fine, can't it?


>              which got into the code unnoticed.

Some patch reviews happened accordingly.


>                                                 pdev will not exist if
> netdev is not allocated, where mucse is netdev private data.

Would you like to reconsider the relevance of another null pointer check?

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Dan Carpenter]

On Sun, Jul 12, 2026 at 08:35:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.
> 
> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> Pass the required address directly to a function call.
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")

There is no NULL dereference here.  It's just pointer math.
No need for a Fixes tag.

regards,
dan carpenter

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
>>
>> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
>> Pass the required address directly to a function call.
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> 
> There is no NULL dereference here.  It's just pointer math.
> No need for a Fixes tag.

How does your view fit to information in an article like “Fun with NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
https://lwn.net/Articles/342330/

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Andrew Lunn]

On Sun, Jul 12, 2026 at 08:35:21PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 12 Jul 2026 20:25:10 +0200
> 
> The address of a data structure member was determined before
> a corresponding null pointer check in the implementation of
> the function “rnpgbe_rm_adapter”.

Please can you show the path taken that mucse is actually NULL.

I tend to agree, that the correct change is to remove the NULL pointer
test.

    Andrew

---
pw-bot: cr

Re: net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Markus Elfring]

>> The address of a data structure member was determined before
>> a corresponding null pointer check in the implementation of
>> the function “rnpgbe_rm_adapter”.
> 
> Please can you show the path taken that mucse is actually NULL.

The applied sanity check indicates a corresponding possibility.

Which result representations would you find helpful for the requested
data flow analysis?


> I tend to agree, that the correct change is to remove the NULL pointer
> test.
Will the omission of such a check gain wider acceptance?

Regards,
Markus

Re: [PATCH net] net: rnpgbe: Pass an expression directly in rnpgbe_rm_adapter()

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]

On Mon, Jul 13, 2026 at 10:22:06AM +0200, Markus Elfring wrote:
> >> The address of a data structure member was determined before
> >> a corresponding null pointer check in the implementation of
> >> the function “rnpgbe_rm_adapter”.
> >>
> >> Thus avoid the risk for undefined behaviour by omitting the variable “hw”.
> >> Pass the required address directly to a function call.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Fixes: 2ee95ec17e97c58b65e978a08b75fa8cb6424e4e ("net: rnpgbe: Add register_netdev")
> > 
> > There is no NULL dereference here.  It's just pointer math.
> > No need for a Fixes tag.
> 
> How does your view fit to information in an article like “Fun with
> NULL pointers, part 1”(by Jonathan Corbet from 2009-07-20)?
> https://lwn.net/Articles/342330/

It does fit, because the problematic code discussed in Jonathan Corbet's
article is of the type:

	int i = ptr->i;

	if (!ptr)
		do_something();

while here we have:

	int *i = &ptr->i;

	if (!ptr)
		do_something();

which at least in my test[1] is relevantly different. Note, I didn't
study the C standard if the compiler is free to optimize out
do_something() also in the 2nd case, but at least today gcc doesn't.

Best regards
Uwe

[1] Me knowing about ARM assembly, that's what I checked:

	$ cat test.c
	#include <stdio.h>
	#include <stdlib.h>

	struct mystruct {
		int i;
		char c;
	};

	int funcdirect(struct mystruct *ptr)
	{
		int i = ptr->i;

		if (!ptr)
			return -1;

		printf("i = %d\n", i);
		return 0;
	}

	int funcindirect(struct mystruct *ptr)
	{
		int *i = &ptr->i;

		if (!ptr)
			return -1;

		printf("i = %d\n", *i);
		return 0;
	}
	$ arm-linux-gnueabihf-gcc -O3 -c test.c
	$ objdump -D test.o

	test.o:     file format elf32-littlearm


	Disassembly of section .text:

	00000000 <funcdirect>:
	   0:	b508      	push	{r3, lr}
	   2:	4603      	mov	r3, r0
	   4:	4803      	ldr	r0, [pc, #12]	@ (14 <funcdirect+0x14>)
	   6:	6819      	ldr	r1, [r3, #0]
	   8:	4478      	add	r0, pc
	   a:	f7ff fffe 	bl	0 <printf>
	   e:	2000      	movs	r0, #0
	  10:	bd08      	pop	{r3, pc}
	  12:	bf00      	nop
	  14:	00000008 	andeq	r0, r0, r8

	00000018 <funcindirect>:
	  18:	b138      	cbz	r0, 2a <funcindirect+0x12>
	  1a:	6801      	ldr	r1, [r0, #0]
	  1c:	4804      	ldr	r0, [pc, #16]	@ (30 <funcindirect+0x18>)
	  1e:	b508      	push	{r3, lr}
	  20:	4478      	add	r0, pc
	  22:	f7ff fffe 	bl	0 <printf>
	  26:	2000      	movs	r0, #0
	  28:	bd08      	pop	{r3, pc}
	  2a:	f04f 30ff 	mov.w	r0, #4294967295	@ 0xffffffff
	  2e:	4770      	bx	lr
	  30:	0000000c 	andeq	r0, r0, ip

So in funcdirect the check is not present, while it is in funcindirect.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]