linux-kernel - Re: [PATCH] kunit: log numbers in decimal and hex

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: <CAGS_qxok+UigJqoROJO2ryUXAWet5V8FL62cn0mr1iuporLu6g@mail.gmail.com>
Date:   Wed, 5 Oct 2022 11:28:27 -0700
From:   Daniel Latypov <dlatypov@...gle.com>
To:     Mark Rutland <mark.rutland@....com>
Cc:     linux-kernel@...r.kernel.org, brendan.higgins@...ux.dev,
        davidgow@...gle.com, kunit-dev@...glegroups.com,
        linux-kselftest@...r.kernel.org
Subject: Re: [PATCH] kunit: log numbers in decimal and hex

On Wed, Oct 5, 2022 at 10:52 AM Mark Rutland <mark.rutland@....com> wrote:
>
> When KUNIT_EXPECT_EQ() or KUNIT_ASSERT_EQ() log a failure, they log the
> two values being compared, with numerical values logged in decimal.
>
> In some cases, decimal output is painful to consume, and hexadecimal
> output would be more helpful. For example, this is the case for tests
> I'm currently developing for the arm64 insn encoding/decoding code,
> where comparing two 32-bit instruction opcodes results in output such
> as:
>
> |  # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791
> |  Expected obj_insn == gen_insn, but
> |      obj_insn == 2332164128
> |      gen_insn == 1258422304
>
> To make this easier to consume, this patch logs the values in both
> decimal and hexadecimal:
>
> |  # test_insn_add_shifted_reg: EXPECTATION FAILED at arch/arm64/lib/test_insn.c:2791
> |  Expected obj_insn == gen_insn, but
> |      obj_insn == 2332164128 (0x8b020020)
> |      gen_insn == 1258422304 (0x4b020020)
>
> As can be seen from the example, having hexadecimal makes it
> significantly easier for a human to spot which specific bits are
> incorrect.
>
> Signed-off-by: Mark Rutland <mark.rutland@....com>
> Cc: Brendan Higgins <brendan.higgins@...ux.dev>
> Cc: David Gow <davidgow@...gle.com>
> Cc: linux-kselftest@...r.kernel.org
> Cc: kunit-dev@...glegroups.com

Acked-by: Daniel Latypov <dlatypov@...gle.com>

This is definitely something that has irked me as well.

I vaguely feel like this can clutter up the output for things that are
just ints, e.g.
    Expected len == 0, but
        len == 2 (0x2)

But I can't think of any suggestions that are actually worth doing [1].
And since this feels like a common enough case, I don't think the
existing workarounds [2] are sufficiently ergonomic.

So I think always showing the hex makes sense.

[1]
E.g. only emitting hex when the number in question is >16, but that
feels too magic (also needs more if's).
E.g. adding in a macro argument/separate set of macros that always format in hex

[2] can use these to coerce hex output
         KUNIT_EXPECT_EQ_MSG(test, obj_insn, gen_insn, "obj=0x%llx, gen=0x%llx",
                             obj_insn, gen_insn);
         KUNIT_EXPECT_PTR_EQ(test, obj_insn, gen_insn);