[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YC08tgirtDsZumkK@chrisdown.name>
Date: Wed, 17 Feb 2021 15:56:38 +0000
From: Chris Down <chris@...isdown.name>
To: Petr Mladek <pmladek@...e.com>
Cc: Johannes Weiner <hannes@...xchg.org>, linux-kernel@...r.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
John Ogness <john.ogness@...utronix.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Steven Rostedt <rostedt@...dmis.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Kees Cook <keescook@...omium.org>, kernel-team@...com
Subject: Re: code style: Re: [PATCH v4] printk: Userspace format enumeration
support
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_?".
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.
Powered by blists - more mailing lists