On 03/07/2026 12:27, Michal Suchánek wrote: > On Fri, Jul 03, 2026 at 11:00:11AM +0100, Mark Rutland wrote: >> On Fri, Jul 03, 2026 at 11:48:49AM +0200, Thomas Gleixner wrote: >>> On Fri, Jul 03 2026 at 09:51, Michal Suchánek wrote: >>>> On Mon, Jun 29, 2026 at 09:05:59PM +0800, Jinjie Ruan wrote: >>>>> - if (secure_computing()) >>>>> + if (!secure_computing()) >>>>> return -1; >>>> Hello, >>>> >>>> I am not fond of this logic inversion. The boolean is meaningless in >>>> itself. >>>> >>>> Previously -1 was used to indicate that the syscall was filtered but you >>>> chose to invert the logic choosing true to mean syscall was not filtered. >>>> >>>> You could choose true to mean that syscall was fitered avoiding this >>>> inversion. >>> That's just wrong. Boolean logic makes more sense with having >>> (!condition()). Just because the old 0/-1 nonsense had it the other way >>> round does not mean it has to stay that way. >> 100% agreed! >> >> Bikeshedding below; sorry. >> >> I think the bigger problem is just that secure_computing() is a terrible >> name that does not express the intended semantic -- it's not clear >> whether "secure computing" means "seccomp permit the syscall" or >> "seccomp is enabled and some special rules now apply" or something else >> entirely. >> >> If we're changing the return type, it might be worth renaming the >> function something like: >> >> seccomp_permits_syscall() > Then not only it is clear which way the boolean value should be > interpreted, it also pervents the accidental inversion of existing > calls. Overall great.
Totally agreed, if we have the opportunity to rename a completely undescriptive function name like "secure_computing" we should take it. - Kevin
