From: Sean Christopherson <seanjc@google.com>
To: angquan yu <angquan21@gmail.com>
Cc: skhan@linuxfoundation.org, shuah@kernel.org, pbonzini@redhat.com,
kvm@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH] Resolve Macro Expansion Warning in nx_huge_pages_test.c
Date: Tue, 28 Nov 2023 17:21:31 -0800 [thread overview]
Message-ID: <ZWaSG4i6dsvj_qvP@google.com> (raw)
In-Reply-To: <20231128221105.63093-1-angquan21@gmail.com>
For the shortlog, specify the scope/domain/namespace of the patch. For KVM
selftests, that's "KVM: selftests:". And ideally, describe the impact of the
change in a more conversational way, as opposed to stating the literal effect of
the patch. Stating that the patch fixes a warning is obviously accurate, but it
doesn't provide any insight as to the severity of the issue, i.e. what's actually
broken and being fixed.
E.g.
KVM: selftests: Actually print out magic token in NX hugepages skip message
On Tue, Nov 28, 2023, angquan yu wrote:
> From: angquan yu <angquan21@gmail.com>
>
> This commit fixes a compiler warning in the file
> x86_64/nx_huge_pages_test.c, which was caused by improper
> macro expansion of '__TEST_REQUIRE'.
>
> Warning addressed:
> - The warning was triggered by the expansion of the '__TEST_REQUIRE'
> macro, indicating a potential issue in how the macro was being
> used or expanded.
>
> Changes made:
> - Modified the usage of the '__TEST_REQUIRE' macro to ensure proper
> expansion. This involved explicitly passing the expected magic token
> (MAGIC_TOKEN) and a descriptive error message to the macro.
> - The fix enhances clarity in the macro usage and ensures that
> the compiler correctly interprets the intended logic, thereby
> resolving the warning.
Fixing warnings is nice, but warnings are a means to an end, i.e. they identify
things that *might* be actual problems. The real issue is that the test will
spit out garbage instead of the magic number.
All that said, applied to kvm-x86 selftests, with a heavily rewritten shortlog
and changelog (I had the same fix locally, so more or less just grabbed what I
had already written).
Thanks!
[1/1] KVM: selftests: Actually print out magic token in NX hugepages skip message
https://github.com/kvm-x86/linux/commit/3b99d46a1170
>
> Signed-off-by: angquan yu <angquan21@gmail.com>
> ---
> tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> index 18ac5c195..323ede6b6 100644
> --- a/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/nx_huge_pages_test.c
> @@ -259,7 +259,8 @@ int main(int argc, char **argv)
> __TEST_REQUIRE(token == MAGIC_TOKEN,
> "This test must be run with the magic token %d.\n"
> "This is done by nx_huge_pages_test.sh, which\n"
> - "also handles environment setup for the test.");
> + "also handles environment setup for the test.",
> + MAGIC_TOKEN);
>
> run_test(reclaim_period_ms, false, reboot_permissions);
> run_test(reclaim_period_ms, true, reboot_permissions);
> --
> 2.39.2
>
prev parent reply other threads:[~2023-11-29 1:21 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-28 22:11 [PATCH] Resolve Macro Expansion Warning in nx_huge_pages_test.c angquan yu
2023-11-29 1:21 ` Sean Christopherson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZWaSG4i6dsvj_qvP@google.com \
--to=seanjc@google.com \
--cc=angquan21@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=shuah@kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.