linux-kernel - Re: [PATCH 3/3] kunit: factor out str constants from binary assertion structs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFd5g45vmGCtnEgG1eQG02q7ayAULgJt3X=2xMma8tAmOH=WDw@mail.gmail.com>
Date:   Thu, 27 Jan 2022 16:46:23 -0500
From:   Brendan Higgins <brendanhiggins@...gle.com>
To:     Daniel Latypov <dlatypov@...gle.com>
Cc:     davidgow@...gle.com, linux-kernel@...r.kernel.org,
        kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org,
        skhan@...uxfoundation.org
Subject: Re: [PATCH 3/3] kunit: factor out str constants from binary assertion structs

On Tue, Jan 25, 2022 at 4:00 PM Daniel Latypov <dlatypov@...gle.com> wrote:
>
> If the compiler doesn't optimize them away, each kunit assertion (use of
> KUNIT_EXPECT_EQ, etc.) can use 88 bytes of stack space in the worst and
> most common case. This has led to compiler warnings and a suggestion
> from Linus to move data from the structs into static const's where
> possible [1].
>
> This builds upon [2] which did so for the base struct kunit_assert type.
> That only reduced sizeof(struct kunit_binary_assert) from 88 to 64.
>
> Given these are by far the most commonly used asserts, this patch
> factors out the textual representations of the operands and comparator
> into another static const, saving 16 more bytes.
>
> In detail, KUNIT_EXPECT_EQ(test, 2 + 2, 5) yields the following struct
>   (struct kunit_binary_assert) {
>     .assert = <struct kunit_assert>,
>     .operation = "==",
>     .left_text = "2 + 2",
>     .left_value = 4,
>     .right_text = "5",
>     .right_value = 5,
>   }
> After this change
>   static const struct kunit_binary_assert_text __text = {
>     .operation = "==",
>     .left_text = "2 + 2",
>     .right_text = "5",
>   };
>   (struct kunit_binary_assert) {
>     .assert = <struct kunit_assert>,
>     .text = &__text,
>     .left_value = 4,
>     .right_value = 5,
>   }
>
> This also DRYs the code a bit more since these str fields were repeated
> for the string and pointer versions of kunit_binary_assert.
>
> Note: we could name the kunit_binary_assert_text fields left/right
> instead of left_text/right_text. But that would require changing the
> macros a bit since they have args called "left" and "right" which would
> be substituted in `.left = #left` as `.2 + 2 = \"2 + 2\"`.
>
> [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ
> [2] https://lore.kernel.org/linux-kselftest/20220113165931.451305-6-dlatypov@google.com/
>
> Signed-off-by: Daniel Latypov <dlatypov@...gle.com>

Reviewed-by: Brendan Higgins <brendanhiggins@...gle.com>