Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

P_LKRG_UNHIDE's behavior inconsistent with its name #251

Open
solardiz opened this issue Nov 28, 2022 · 2 comments
Open

P_LKRG_UNHIDE's behavior inconsistent with its name #251

solardiz opened this issue Nov 28, 2022 · 2 comments

Comments

@solardiz
Copy link
Contributor

We wrap some functionality related to the hide feature/setting in #ifdef P_LKRG_UNHIDE and define that macro by default. Changing that default (not defining the macro) should probably remove the ability to unhide. What it actually does is remove the code to unhide and the hide setting, which remains disabled by default, so in essence it removes the ability to hide.

For the existence of P_LKRG_UNHIDE to make sense, undefining it should probably preserve the hide setting, but disallow disabling it (once enabled). Alternatively, hide should be enabled by default (at least in builds without P_LKRG_UNHIDE).

@solardiz
Copy link
Contributor Author

The sysctl setting used to be unhide prior to e5924c4 in 2017, so prior to public release of LKRG. Changing it to hide back then is what made P_LKRG_UNHIDE inconsistent with its name. The easiest fix for now would be to also rename P_LKRG_UNHIDE to P_LKRG_HIDE, but I'm not sure that's what we want long-term.

@solardiz
Copy link
Contributor Author

Disabling P_LKRG_UNHIDE currently leaves p_hide_itself compiled in, and there's a call from p_lkrg_register:

   if (P_CTRL(p_hide_lkrg)) {
      p_hide_itself();
   }

However, there's nothing to enable that setting in such a build. So we have dead code. If we rename to P_LKRG_HIDE, we should also exclude that dead code in builds with P_LKRG_HIDE disabled.

Alternatively, we can add a way to set p_hide_lkrg in such builds - add a module parameter or/and make it default to enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant