ktrace: Record cpuset violations with KTR_CAPFAIL

[jfree]

Summary

Report Capsicum violations in the cpuset namespace with CAPFAIL_CPUSET.

Programs that are not yet Capsicumized may be traced to discover
potential capability failures. With `ktrace -t p` and kdump, you
are given a list of syscalls as a starting point for program
Capsicumization.

[markj]

sys/kern/kern_cpuset.c

1803

Rather than duplicating all of these conditions, I'd suggest lifting them into a function bool cpuset_capmode_allowed(level, which, id) and call that from this function.

[jfree]

It feels wrong to return inside of a macro, but I did not see any comments about it in style(9). This seems to be the solution that is most elegant in minimizing code duplication. Let me know your thoughts.

[jfree]

Actually, scratch that. I just understood your comment and this is a better solution.

As a side note though... Is returning in macros usually frowned upon?

[markj]

In D40677#988077, @jfree wrote:

Actually, scratch that. I just understood your comment and this is a better solution.

As a side note though... Is returning in macros usually frowned upon?

I would frown at that. :) It introduces hidden control flow which makes it hard to see quickly what a function does.

Consider the common case where a function allocates some memory and is supposed to free it before returning: if I'm reviewing the code and want to verify it behaves properly with respect to that free() call, it's much easier to check if I just have to look for the "return" keyword in the function.

Once in a while it's handy, but it should be avoided if possible.

sys/kern/kern_cpuset.c

1793

Note that you've factored the caller such that cpuset_capmode_allowed() isn't really required now: you could inline it into the caller without introducing code duplication.

But I would keep this as it is: cpuset_capmode_allowed() is a pure function, and IMO it's good style to factor those out when possible. (cpuset_check_capabilities() itself is not a pure function because it may call ktrcapfail(), which logs to a ktrace buffer.)

[jfree]

In D40677#988214, @markj wrote:

I would frown at that. :) It introduces hidden control flow which makes it hard to see quickly what a function does.

Consider the common case where a function allocates some memory and is supposed to free it before returning: if I'm reviewing the code and want to verify it behaves properly with respect to that free() call, it's much easier to check if I just have to look for the "return" keyword in the function.

Once in a while it's handy, but it should be avoided if possible.

Thanks for the explanation. These things seem obvious once you know them, but it is sometimes hard to reason if you don't. I think in most cases, it is possible to rearrange a function to remove a macro that returns anyway. That's what lead me to explore other solutions.