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

["Uwe Kleine-König" <u.kleine-koenig@baylibre.com>]

[-- 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 --]