Re: [PATCH v6 3/4] printk: Userspace format indexing support

From: Chris Down
Date: Tue May 18 2021 - 12:28:37 EST


Andy Shevchenko writes:
As for the literals, are you saying that you prefer that it's symbolised as
a macro or static char, or do you know of an API where this kind of name can
be canonically accessed?

I have heard that modern GCC (at least) can utilize same constant literals in a
single compilation unit, so it won't be duplicated.

But more serious here is the guarantees of the name. Shouldn't it come from
KBuild / Makefile into some header like version do?

I'm not against that, but it seems like something worth doing outside of this patch series, unless you have strong feelings to the contrary?

More than happy to put that on my TODO list for tree-wide cleanups.

> > +#define seq_escape_printf_format(s, src) \
> > + seq_escape_str(s, src, ESCAPE_ANY | ESCAPE_NAP | ESCAPE_APPEND, "\"\\")
>
> Hmm... But after your ESCAPE_SPECIAL update why " is in @only?
> Not sure about back slash either.

Good question! It's because ESCAPE_NAP (used to reduce scope of
ESCAPE_OCTAL) will cause double quote and backslash to be ignored for
quoting otherwise, even with ESCAPE_SPECIAL from ESCAPE_ANY.

Ah, makes sense. Yep, it's a bit complicated, but okay, perhaps it needs a
comment near to the macro.

Sure thing, will do.

> > +static int __init pi_init(void)

> No __exit? (There is a corresponding call for exit)

Hmm, can't printk only be built in to the kernel, so it can't be unloaded?
At least it looks that way from Kconfig. Maybe I'm missing something and
there's some other way that might be invoked?

While it's true, it may help in these cases:
1) getting things done in a clean way
2) finding bugs during boot cycle
3) (possibly) making better debugging in virtual environments
4) (also possibly) clean up something which shouldn't be seen by the next
(unsecure) kernel, like kexec.

I'm not sure about these, but it what comes to my mind.

Hmm. Petr, what do you think? :-)