Re: [PATCH] x86/tools: fix line number reported for malformed lines

[Miguel Ojeda]

On Wed, Feb 21, 2024 at 10:00 AM Valentin Obst <kernel@valentinobstde> wrote:
>
> While debugging the `X86_DECODER_SELFTEST` failure first reported in [1],
> [In this case the line causing the failure is interpreted as two lines by
> the tool (due to its length, but this is fixed by [1, 2]), and the second
> line is reported. Still the spatial closeness between the reported line and
> the line causing the failure would have made debugging a lot easier.]

Thanks Valentin, John et al. for digging into this (and the related
issue) -- very much appreciated.

It looks good to me:

Reviewed-by: Miguel Ojeda <ojeda@xxxxxxxxxx>
Tested-by: Miguel Ojeda <ojeda@xxxxxxxxxx>

This should probably have a Fixes tag -- from a quick look, the
original test did not seem to have the problem because `insns` was
equivalent to the number of lines since there was no `if ... {
continue; }` for the symbol case. At some point that branch was added,
so that was not true anymore, thus that one should probably be the
Fixes tag, though please double-check:

    Fixes: 35039eb6b199 ("x86: Show symbol name if insn decoder test failed")

It is a minor issue for sure, so perhaps not worth backporting, but
nevertheless the hash is in a very old kernel, and thus the issue
applies to all stable kernels. So it does not hurt flagging it to the
stable team:

    Cc: stable@xxxxxxxxxxxxxxx

In addition, John reported the original issue, but this one was found
due to that one, and I am not exactly sure who did what here. Please
consider whether one of the following (or similar) may be fair:

    Reported-by: John Baublitz <john.m.baublitz@xxxxxxxxx>
    Debugged-by: John Baublitz <john.m.baublitz@xxxxxxxxx>

An extra Link to the discussion in Zulip could be nice too:

    Link: https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/insn_decoder_test.20failure/near/421075039

Finally, a nit: links are typically written like the following -- you
can still use bracket references at the end:

    Link: https://lore.kernel.org/lkml/Y9ES4UKl%2F+DtvAVS@xxxxxxxxx/T/ [1]
    Link: https://lore.kernel.org/rust-for-linux/20231119180145.157455-1-sergio.collado@xxxxxxxxx/
[2]

Cheers,
Miguel