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:
pgpjBEPhoTVYw.pgp
Description: OpenPGP digital signature
