[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240307223849.13d5b58b@barney>
Date: Thu, 7 Mar 2024 22:38:49 +0100
From: Michael Büsch <m@...s.ch>
To: Rand Deeb <rand.sec96@...il.com>
Cc: deeb.rand@...fident.ru, jonas.gorski@...il.com, khoroshilov@...ras.ru,
kvalo@...nel.org, linux-kernel@...r.kernel.org,
linux-wireless@...r.kernel.org, lvc-project@...uxtesting.org,
voskresenski.stanislav@...fident.ru
Subject: Re: [PATCH v3] ssb: Fix potential NULL pointer dereference in
ssb_device_uevent
On Fri, 8 Mar 2024 00:19:28 +0300
Rand Deeb <rand.sec96@...il.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/
Content of type "application/pgp-signature" skipped
Powered by blists - more mailing lists