Hello everybody,
https://review.coreboot.org/c/coreboot/+/51825 proposes getting rid of the rule to make if-statement blocks (and the like) as short as possible. The rationale is to encourage a style that avoids subtle bugs which then need to be found by tooling such as Coverity Scan and fixed by commits like https://review.coreboot.org/51786.
Julius disagrees and requests wider discussion, specifically on the mailing list. So here we are: Arguments? Opinions?
Julius, maybe you want to make your case, I didn't want to risk representing a distorted position.
Regards, Patrick
Hello! Actually folks I am going to support both ones. There are reasons, but I'm not prepared to go into them now. ----- Gregg C Levine gregg.drwho8@gmail.com "This signature fought the Time Wars, time and again."
On Thu, Mar 25, 2021 at 6:23 PM Patrick Georgi via coreboot coreboot@coreboot.org wrote:
Hello everybody,
https://review.coreboot.org/c/coreboot/+/51825 proposes getting rid of the rule to make if-statement blocks (and the like) as short as possible. The rationale is to encourage a style that avoids subtle bugs which then need to be found by tooling such as Coverity Scan and fixed by commits like https://review.coreboot.org/51786.
Julius disagrees and requests wider discussion, specifically on the mailing list. So here we are: Arguments? Opinions?
Julius, maybe you want to make your case, I didn't want to risk representing a distorted position.
Regards, Patrick -- Google Germany GmbH, ABC-Str. 19, 20354 Hamburg Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
https://review.coreboot.org/c/coreboot/+/51825 proposes getting rid of the rule to make if-statement blocks (and the like) as short as possible. The rationale is to encourage a style that avoids subtle bugs which then need to be found by tooling such as Coverity Scan and fixed by commits like https://review.coreboot.org/51786.
My main case is really just that I think the whole premise is incorrect: these kinds of errors can be found by checkpatch, at upload time. There should be no risk of them slipping into the code and thus we don't need to add any inconvenience for humans to prevent something that can already be automatically be prevented by a computer.
I've double-checked that now and realized that checkpatch currently doesn't look for this like I thought it did. But there is a fix, and it works very well (I've found no real false-positives in the whole coreboot tree): https://review.coreboot.org/c/coreboot/+/51838. I am also (re-)submitting this to Linux so hopefully we wouldn't be out of sync for too long if we want to apply it immediately.
-
Gregg Levine -
Julius Werner -
Patrick Georgi