linux-kernel - Re: [PATCH v3 0/2] kunit: add macro to allow conditionally exposing static symbols to tests

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CABVgOS==niZi5O4Gk8uxHdnmX4YFs2e6XYG=B+SQ6Po6v6bHkg@mail.gmail.com>
Date:   Wed, 7 Dec 2022 10:54:43 +0800
From:   David Gow <davidgow@...gle.com>
To:     John Johansen <john.johansen@...onical.com>
Cc:     Rae Moar <rmoar@...gle.com>, brendanhiggins@...gle.com,
        dlatypov@...gle.com, skhan@...uxfoundation.org,
        tales.aparecida@...il.com, kunit-dev@...glegroups.com,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        apparmor@...ts.ubuntu.com
Subject: Re: [PATCH v3 0/2] kunit: add macro to allow conditionally exposing
 static symbols to tests

On Wed, Dec 7, 2022 at 10:43 AM John Johansen
<john.johansen@...onical.com> wrote:
>
> On 12/6/22 17:40, Rae Moar wrote:
> > Currently in order to test a static function, tests must be included in the
> > same translation unit as the function. However, this can cause issues with
> > including implementation and test code in the same file. As an alternative,
> > the first patch in this series creates a macro that will set a function to
> > be static or not depending on whether CONFIG_KUNIT is enabled. This allows
> > the function to be visible during testing and static otherwise.
> >
> > As an example, the current status quo to test static functions is:
> >
> > === test.c ===
> >
> > static void test_case(struct kunit *test)
> > {
> >    KUNIT_EXPECT_EQ(test, my_func_to_test(), 2);
> > }
> >
> > Then the tests are included in the implementation file as a workaround to
> > the issue of testing static functions:
> >
> > === implementation.c ===
> >
> > static int my_func_to_test() {...}
> > ...
> > #include "test.c"
> >
> > Instead, the function could be defined with this new macro:
> >
> > === implementation.c ===
> >
> > VISIBLE_IF_KUNIT int my_func_to_test() {...}
> >
> > The first patch also creates a macro that will export a symbol into a kunit
> > testing namespace only if CONFIG_KUNIT is enabled. This follows the logic
> > above and allows symbols to be conditionally exported based on the testing
> > status.
> >
> > The second patch in the series updates the policy_unpack test in AppArmor
> > to show an example of how to use both of these macros in order to address
> > the issue of testing static functions. Additionally, the patch allows the
> > policy_unpack test to be built as a module.
> >
> > Changes since v2:
> >   - Add mention of namespacing symbols to the commit message of the
> >     second patch.
> >   - Change module name in the second patch from policy_unpack_test to
> >     apparmor_policy_unpack_test.
> >
> > Changes since v1:
> >   - Changed the namespace of exported symbols for the apparmor
> >     policy_unpack_test by adding the aa_ prefix.
> >   - Separated the documentation comments for macros in
> >     include/kunit/visibility.h.
> >   - Changed copyright date and author for include/kunit/visibility.h.
> >
> > Rae Moar (2):
> >    kunit: add macro to allow conditionally exposing static symbols to
> >      tests
> >    apparmor: test: make static symbols visible during kunit testing
> >
> >   include/kunit/visibility.h                |  33 +++
> >   security/apparmor/Kconfig                 |   4 +-
> >   security/apparmor/Makefile                |   3 +
> >   security/apparmor/include/policy_unpack.h |  50 +++++
> >   security/apparmor/policy_unpack.c         | 238 ++++++++++------------
> >   security/apparmor/policy_unpack_test.c    |  69 ++++---
> >   6 files changed, 229 insertions(+), 168 deletions(-)
> >   create mode 100644 include/kunit/visibility.h
> >
> >
> > base-commit: 0f08f3e2a0186dfb8e33cb46105228eb18448a0e
>
> thanks Rae,
>
> looks good to me, David unless you tell me otherwise I assume this is
> still going in via the kselftest/kunit tree.
>

Yup, both of these are looking good to me (and work fine on my machine).

I've added these to the queue for the kselftest/kunit tree. There's a
chance they won't make it into 6.2, but if not, they should be the
first things in the 6.3 branch.

Thanks very much,
-- David

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)