Messages in this thread | | | Date | Thu, 18 Feb 2021 11:58:31 +0100 | From | Petr Mladek <> | Subject | Re: code style: Re: [PATCH v4] printk: Userspace format enumeration support |
| |
On Wed 2021-02-17 15:56:38, Chris Down wrote: > Petr Mladek writes: > > > > How about config PRINTK_INDEX? > > > > > > Ah yes, I also like that. PRINTK_INDEX is fine from my perspective and is > > > more straightforward than "enumeration", thanks. > > > > It is better than enumeration. But there is still the same > > problem. The word "index" is used neither in the code > > nor in the debugfs interface. It is like enabling cars and > > seeing apples. > > > > What about CONFIG_PRINTK_DEBUGFS? > > > > It seems that various subsystems use CONFIG_<SUBSYSTEM>_DEBUGFS > > pattern when they expose some internals in debugfs. > > The thing I don't like about that is that it describes a largely > inconsequential implementation detail rather than the semantic intent of the > config change, which is what the person deciding what to include in their > config is likely to care about. Often when I see "XXX debug interface" when > doing `make oldconfig` I think to myself "yes, but what does the debugfs > interface _do_?".
I see.
> If someone else was writing this patch, and I saw "CONFIG_PRINTK_DEBUGFS" > appear in my prod kernel, I'd probably say N, because I don't need printk > debugging information. On the other hand, if I saw "CONFIG_PRINTK_INDEX", I'd > immediately understand that it's probably applicable to me. > > I'm happy to rename the debugfs structure as <debugfs>/printk/fmt_index if it > helps, but personally I really feel CONFIG_PRINTK_{INDEX,ENUMERATION,CATALOGUE} > is a lot more descriptive than just saying "it has a debugfs interface" in the > config name for that reason.
PRINTK_INDEX sounds the best to me. Keep in mind that I am not a native speaker.
And my concern will be gone when we use it also in the API and debugfs hierarchy as suggested by Johannes.
Another compromise might be to have CONFIG_PRINTK_FORMATS_INDEX. Then the prefix printk_format_, pf_ would still match the option. Or we could use printk_format_index_m, pfi_ indexes.
Best Regards, Petr
PS: I feel that I have enough bike-shading. I think that I will be fine with anything that you choose ;-)
| |