Re: [PATCH v3] ssb: Fix potential NULL pointer dereference in ssb_device_uevent

[Michael Büsch]

On Fri,  8 Mar 2024 00:19:28 +0300
Rand Deeb <rand.sec96@xxxxxxxxx> wrote:

> Yes, I agree, this is not critical code, but what's the point of leaving 
> redundant conditions even if they won't make a significant performance 
> difference, regardless of the policy (In other words, as a friendly 
> discussion) ?

The point is that leaving them in is defensive programming against future changes
or against possible misunderstandings of the situation.

Removing this check would improve nothing.

> I understand and respect your point of view as software engineer but it's a
> matter of design problems which is not our case here.

No, it very well is.

> Defensive programming is typically applied when there's a potential risk, 

A NULL pointer dereference is Undefined Behavior.
It can't get much worse in C.

> If we adopt this
> approach as a form of defensive programming, we'd find ourselves adding 
> similar conditions to numerous functions and parameters.

Not at all.
Your suggestion was about REMOVING a null pointer check.
Not about adding one.
I NAK-ed the REMOVAL of a null pointer check. Not the addition.

> Moreover, this 
> would unnecessarily complicate the codebase, especially during reviews.

Absolutely wrong.
Not having a NULL check complicates reviews.
Reviewers will have to prove that pointers cannot be NULL, if there is no check.

> so would you recommend fix the commit message as Jeff Johnson recommended ?
> or just keep it as it is ?

I don't care about the commit message.
I comment on the change itself.

-- 
Michael Büsch
https://bues.ch/

Attachment: pgpeEYTRICkSj.pgp
Description: OpenPGP digital signature