Re: [PATCH v1] printk: add dummy vprintk_emit for when CONFIG_PRINTK=n

[Brendan Higgins <brendanhiggins@google.com>]

On Wed, Aug 28, 2019 at 12:02:31PM +0900, Sergey Senozhatsky wrote:
> On (08/27/19 16:48), Brendan Higgins wrote:
> > Previously vprintk_emit was only defined when CONFIG_PRINTK=y, this
> > caused a build failure in kunit/test.c when CONFIG_PRINTK was not set.
> > Add a no-op dummy so that callers don't have to ifdef around this.
> > 
> > Note: It has been suggested that this go in through the kselftest tree
> > along with the KUnit patches, because KUnit depends on this. See the
> > second link for the discussion on this.
> 
> Is there any reason for kunit to use vprintk_emit()? Can you switch
> to pr_err()/pr_info()/pr_foo()?
> 
> vprintk_emit() function is pretty internal. It's not static because
> drivers/base/core.c wants to add some extra payload to printk()
> messages (extended headers, etc).

I actually use it in a very similar way as dev_printk() does. I am using
it to define an equivalent kunit_printk(), which takes a log level, and
adds its own test information to the log.

What I have now is:

static int kunit_vprintk_emit(int level, const char *fmt, va_list args)
{
	return vprintk_emit(0, level, NULL, 0, fmt, args);
}

static int kunit_printk_emit(int level, const char *fmt, ...)
{
	va_list args;
	int ret;

	va_start(args, fmt);
	ret = kunit_vprintk_emit(level, fmt, args);
	va_end(args);

	return ret;
}

static void kunit_vprintk(const struct kunit *test,
			  const char *level,
			  struct va_format *vaf)
{
	kunit_printk_emit(level[1] - '0', "\t# %s: %pV", test->name, vaf);
}

The closest thing I can do without vprintk_emit is:

static void kunit_vprintk(const struct kunit *test,
			  const char *level,
			  struct va_format *vaf)
{
	printk("%s\t# %s: %pV", level, test->name, vaf);
}

But checkpatch complains:

WARNING: printk() should include KERN_<LEVEL> facility level

Based on the printk() implementation, it looks like it should be fine to
provide the level via format string since the formatting is performed
before the log level is checked; nevertheless, it seemed to me like
vprintk_emit was closer to what I wanted (again, it's what dev_printk
uses). Shuah and Tim seemed to agree with me:

https://lore.kernel.org/linux-kselftest/ECADFF3FD767C149AD96A924E7EA6EAF977A5D82@USCULXMSG01.am.sony.com/

Nevertheless, I probably don't want add any custom dict entries.

Really, what I think I want is a printk_level().

Thoughts?