On Fri, Sep 21, 2018 at 03:55:33PM -0400, Mimi Zohar wrote: > On Fri, 2018-09-21 at 12:33 -0700, Eric Biggers wrote: > > On Fri, Sep 21, 2018 at 03:02:14PM -0400, Mimi Zohar wrote: > > > On Fri, 2018-09-21 at 11:54 -0700, Eric Biggers wrote: > > > > On Fri, Sep 21, 2018 at 02:42:38PM -0400, Mimi Zohar wrote: > > > > > On Fri, 2018-09-07 at 13:25 -0700, Eric Biggers wrote: > > > > > > From: Eric Biggers <ebiggers@xxxxxxxxxx> > > > > > > > > > > > > The 'init_keyring' variable actually just gave the value of > > > > > > CONFIG_INTEGRITY_TRUSTED_KEYRING. We should check the config option > > > > > > directly instead. No change in behavior; this just simplifies the code. > > > > > > > > > > We try to minimize as much as possible "ifdefs" in C code. This > > > > > change is moving in the wrong direction. > > > > > > > > So your preferred approach is to store the values of Kconfig options in > > > > variables? That defeats much of the point of having Kconfig options... > > > > > > No, I prefer using "ifdefs" in include files, not C code, and defining > > > stub functions. > > > > > > Mimi > > > > > > > integrity_init_keyring() is already stubbed out in a header. What are you > > suggesting, exactly? > > Refer to section "20) Conditional Compilation" of > Documentation/process/coding-style.rst. > > Mimi > I'm already familiar with that. Unfortunately, you haven't clearly indicated what alternative you prefer, and it's unclear whether you've even read my patch, given that you're apparently saying to define a stub function which actually already exists. Maybe you want the 'init_keyring' bool replaced IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING)? That doesn't really make sense though, because integrity_init_keyring() is already stubbed out in some configurations; it makes more sense to fix the condition for stubbing it out... - Eric
