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

["Michael Büsch" <m@bues.ch>]

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

On Fri,  8 Mar 2024 00:19:28 +0300
Rand Deeb <rand.sec96@gmail.com> 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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]