On Thu, Mar 7, 2024 at 9:24 PM Michael Büsch <m@xxxxxxx> wrote: > There is only one reason to eliminate NULL checks: > In extremely performance critical code, if it improves performance > significantly and it is clearly documented that the pointer can never be NULL. > > This is not that case here. This is not critical code. Hi Michael, thank you for your collaboration and feedback. 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) ? Please take a look at https://git.kernel.org/netdev/net-next/c/92fc97ae9cfd same situation but it has been applied ! why ? > Having NULL checks is defensive programming. > Removing NULL checks is a hazard. > Not having these checks is a big part of why security sucks in today's software. 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. Defensive programming is typically applied when there's a potential risk, but in our scenario, it's impossible for 'dev' to be NULL. If we adopt this approach as a form of defensive programming, we'd find ourselves adding similar conditions to numerous functions and parameters. Moreover, this would unnecessarily complicate the codebase, especially during reviews. For instance, encountering such a condition might lead one to assume that 'dev' could indeed be NULL before reaching this function. That's my viewpoint. > V3 shall be applied, because it fixes a clear bug. Whether this bug can actually > be triggered or not in today's kernel doesn't matter. so would you recommend fix the commit message as Jeff Johnson recommended ? or just keep it as it is ? -- Best Regards Rand Deeb
