Hi Petter, On Fri, 17 Jan 2025 16:54:33 +0100, Petter Reinholdtsen wrote: > [Jean Delvare] > > I don't understand the test for (ofmt != NULL) in pr_finish(). I can't > > see how ofmt could not be set, if that was really possible then I > > believe things would break much earlier. > > > > I also don't understand why you explicitly initialized ofmt to NULL, > > this shouldn't be needed. > > I can not really speak for Jiri, but to me such approaches are good > programming practice and what is commonly known as defensive > programming, <URL: https://en.wikipedia.org/wiki/Defensive_programming >. > > I just wanted to air a small defence against the practice, as it in my > experience defense against future surprises when many people update a > code base. :)
It's not like dmidecode is a very active project with hundreds of developers ;-) I'm very much in favor of defensive programming on interfaces (library, user input, kernel/user-space...) I'm also in favor of defensive programming for services - you definitely don't want the service to crash for everyone due to the action of a single user. I am, however, not in favor of defensive programming to catch programming errors in a command-line tool for which I have a non-regression test suite. Programming errors will be caught early and fixed early, with the help of gdb, without the need to build and include unused code in every dmidecode binary forever. Note that preventing a segmentation fault actually prevents from using gdb to investigate the issue, potentially making the investigation much harder. In particular, if you test for a condition which should always be true, but don't report anything in case it is false, you are effectively silencing the bug, making it much harder to spot, investigate and fix. If you really want to test the condition, then IMHO you should either use assert() or a custom error message to report the problem. In other words, if you do defensive programming, do it the right way. Anyway, this was only a suggestion, that's not a blocker. I appreciate Jiri's work (and wish I could review it faster - this is still in progress ATM) and having JSON output support in dmidecode soon is more important than my preferences about defensive programming. -- Jean Delvare SUSE L3 Support
