On Wed, Feb 08, 2023 at 08:55:19PM +0000, Mark Rutland wrote:
> On Wed, Feb 08, 2023 at 09:14:53PM +0100, Peter Zijlstra wrote:
> > On Wed, Feb 08, 2023 at 07:17:15AM -0800, Dave Hansen wrote:
> > > On 2/6/23 04:49, Ard Biesheuvel wrote:
> > > > --- a/arch/x86/kernel/apm_32.c
> > > > +++ b/arch/x86/kernel/apm_32.c
> > > > @@ -609,7 +609,7 @@ static long __apm_bios_call(void *_call)
> > > >
> > > > apm_irq_save(flags);
> > > > firmware_restrict_branch_speculation_start();
> > > > - ibt = ibt_save();
> > > > + ibt = ibt_save(true);
> > >
> > > My only nit with these is the bare use of 'true'/'false'. It's
> > > impossible to tell at the call-site what the 'true' means. So, if you
> > > happen to respin these and see a nice way to remedy this I'd appreciate it.
> >
> > I've often wished for a named argument extention to C, much like named
> > initializers, such that one can write:
> >
> > ibt_save(.disable = true);
> >
> > Because the thing you mention is very common with boolean arguments, the
> > what gets lost in the argument name and true/false just isn't very
> > telling.
> >
> > But yeah, even if by some miracle all compiler guys were like, YES! and
> > implemented it tomorrow, we couldn't use it for a good few years anyway
> > :-/
>
> Well... ;)
>
> | [mark@lakrids:~]% cat args.c
> | #include <stdbool.h>
> | #include <stdio.h>
> |
> | struct foo_args {
> | bool enable;
> | unsigned long other;
> | };
> |
> | void __foo(struct foo_args args)
> | {
> | printf("foo:\n"
> | " enable: %s\n"
> | " other: 0x%lx\n",
> | args.enable ? "YES" : "NO",
> | args.other);
> | }
> |
> | #define foo(args...) \
> | __foo((struct foo_args) { args })
> |
> |
> | int main(int argc, char *argv[])
> | {
> | foo(true);
> | foo(.enable = true);
> | foo(false, .other=0xdead);
> | }
> | [mark@lakrids:~]% gcc args.c -o args
> | [mark@lakrids:~]% ./args
> | foo:
> | enable: YES
> | other: 0x0
> | foo:
> | enable: YES
> | other: 0x0
> | foo:
> | enable: NO
> | other: 0xdead
I am horrified and delighted. And the resulting codegen is identical:
https://godbolt.org/z/eKTMPYc17
Without this fancy solution, what I'd seen is just using an enum:
enum do_the_thing {
THING_DISABLE = 0,
THING_ENABLE,
};
void foo(enum do_the_thing enable)
{
if (enable) { ... }
}
foo(THING_ENABLE);
--
Kees Cook
