linux-kernel - Re: [PATCH v3] kunit: fix failure to build without printk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFd5g479vsN1M24SNgpH4GDj5XK0yAvLsUpA0s6NqfCtTT+isQ@mail.gmail.com>
Date:   Tue, 3 Sep 2019 16:42:05 -0700
From:   Brendan Higgins <brendanhiggins@...gle.com>
To:     Joe Perches <joe@...ches.com>
Cc:     shuah <shuah@...nel.org>, kunit-dev@...glegroups.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Petr Mladek <pmladek@...e.com>, sergey.senozhatsky@...il.com,
        Steven Rostedt <rostedt@...dmis.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        "Bird, Timothy" <Tim.Bird@...y.com>
Subject: Re: [PATCH v3] kunit: fix failure to build without printk

On Tue, Sep 3, 2019 at 4:35 PM Joe Perches <joe@...ches.com> wrote:
>
> On Tue, 2019-09-03 at 16:21 -0700, Brendan Higgins wrote:
> > Previously KUnit assumed that printk would always be present, which is
> > not a valid assumption to make. Fix that by removing call to
> > vprintk_emit, and calling printk directly.
> >
> > This fixes a build error[1] reported by Randy.
> >
> > For context this change comes after much discussion. My first stab[2] at
> > this was just to make the KUnit logging code compile out; however, it
> > was agreed that if we were going to use vprintk_emit, then vprintk_emit
> > should provide a no-op stub, which lead to my second attempt[3]. In
> > response to me trying to stub out vprintk_emit, Sergey Senozhatsky
> > suggested a way for me to remove our usage of vprintk_emit, which led to
> > my third attempt at solving this[4].
> >
> > In my previous version of this patch[4], I completely removed
> > vprintk_emit, as suggested by Sergey; however, there was a bit of debate
> > over whether Sergey's solution was the best. The debate arose due to
> > Sergey's version resulting in a checkpatch warning, which resulted in a
> > debate over correct printk usage. Joe Perches offered an alternative fix
> > which was somewhat less far reaching than what Sergey had suggested and
> > importantly relied on continuing to use %pV. Much of the debated
> > centered around whether %pV should be widely used, and whether Sergey's
> > version would result in object size bloat. Ultimately, we decided to go
> > with Sergey's version.
> >
> > Reported-by: Randy Dunlap <rdunlap@...radead.org>
> > Link[1]: https://lore.kernel.org/linux-kselftest/c7229254-0d90-d90e-f3df-5b6d6fc0b51f@infradead.org/
> > Link[2]: https://lore.kernel.org/linux-kselftest/20190827174932.44177-1-brendanhiggins@google.com/
> > Link[3]: https://lore.kernel.org/linux-kselftest/20190827234835.234473-1-brendanhiggins@google.com/
> > Link[4]: https://lore.kernel.org/linux-kselftest/20190828093143.163302-1-brendanhiggins@google.com/
> > Cc: Stephen Rothwell <sfr@...b.auug.org.au>
> > Cc: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
> > Cc: Joe Perches <joe@...ches.com>
> > Cc: Tim.Bird@...y.com
> > Signed-off-by: Brendan Higgins <brendanhiggins@...gle.com>
> > Acked-by: Randy Dunlap <rdunlap@...radead.org> # build-tested
> > Reviewed-by: Petr Mladek <pmladek@...e.com>
> > ---
> >
> > Sorry for the long commit message, but given the long discussion (and
> > some of the confusion that occurred in the discussion), it seemed
> > appropriate to summarize the discussion around this patch up to this
> > point (especially since one of the proposed patches was under a separate
> > patch subject).
> >
> > No changes have been made to this patch since v2, other than the commit
> > log.
> []
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> []
> > @@ -339,9 +339,8 @@ static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp)
> >
> >  void kunit_cleanup(struct kunit *test);
> >
> > -void __printf(3, 4) kunit_printk(const char *level,
> > -                              const struct kunit *test,
> > -                              const char *fmt, ...);
> > +#define kunit_print_level(KERN_LEVEL, test, fmt, ...) \
> > +     printk(KERN_LEVEL "\t# %s: " fmt, (test)->name, ##__VA_ARGS__)
>
> Non trivial notes:
>
> Please do not use KERN_LEVEL as a macro argument.
> It would just be a source of possible confusion.
>
> Please use level or lvl like nearly every other macro
> that does this uses.

Will do.

> And there is nothing wrong with using kunit_printk and it's
> not necessary to use an odd name like kunit_printk_level.

Sounds reasonable.

[...]

Thanks!