Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: Amit Vadhavana <av2082000@gmail.com>,
	paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
	casey@schaufler-ca.com, shuah@kernel.org
Cc: ricardo@marliere.net, linux-kernel-mentees@lists.linux.dev,
	linux-security-module@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] selftests: lsm: Refactor `flags_overset_lsm_set_self_attr` test
Date: Thu, 14 Nov 2024 09:25:09 -0700	[thread overview]
Message-ID: <52cc8e51-9e85-465b-8ee3-63a7a0a42951@linuxfoundation.org> (raw)
In-Reply-To: <20241112182810.24761-1-av2082000@gmail.com>

On 11/12/24 11:28, Amit Vadhavana wrote:
> - Remove unnecessary `tctx` variable, use `ctx` directly.
> - Simplified code with no functional changes.
> 

I would rephrase the short to simply say Remove unused variable,
as refactor implies more extensive changes than what this patch
is actually doing.

Please write complete sentences instead of bullet points in the
change log.

How did you find this problem? Do include the details on how
in the change log.

> Signed-off-by: Amit Vadhavana <av2082000@gmail.com>
> ---
>   tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> index 66dec47e3ca3..732e89fe99c0 100644
> --- a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> +++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> @@ -56,16 +56,15 @@ TEST(flags_zero_lsm_set_self_attr)
>   TEST(flags_overset_lsm_set_self_attr)
>   {
>   	const long page_size = sysconf(_SC_PAGESIZE);
> -	char *ctx = calloc(page_size, 1);
> +	struct lsm_ctx *ctx = calloc(page_size, 1);

Why not name this tctx and avoid changes to the ASSERT_EQs
below?

>   	__u32 size = page_size;
> -	struct lsm_ctx *tctx = (struct lsm_ctx *)ctx;
>   
>   	ASSERT_NE(NULL, ctx);
>   	if (attr_lsm_count()) {
> -		ASSERT_LE(1, lsm_get_self_attr(LSM_ATTR_CURRENT, tctx, &size,
> +		ASSERT_LE(1, lsm_get_self_attr(LSM_ATTR_CURRENT, ctx, &size,
>   					       0));
>   	}
> -	ASSERT_EQ(-1, lsm_set_self_attr(LSM_ATTR_CURRENT | LSM_ATTR_PREV, tctx,
> +	ASSERT_EQ(-1, lsm_set_self_attr(LSM_ATTR_CURRENT | LSM_ATTR_PREV, ctx,
>   					size, 0));
>   
>   	free(ctx);

You have to change this tctx for sure.

With these changes:

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

Paul, James,

Please do let me know if you would me to take this through
kselftest tree.

thanks,
-- Shuah



  parent reply	other threads:[~2024-11-14 16:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-12 18:28 [PATCH] selftests: lsm: Refactor `flags_overset_lsm_set_self_attr` test Amit Vadhavana
2024-11-12 18:45 ` Casey Schaufler
2024-11-14 16:25 ` Shuah Khan [this message]
2024-11-14 16:55   ` Casey Schaufler
2024-11-14 17:08     ` Shuah Khan
2024-11-16 15:25       ` Amit
2024-11-27  3:38   ` Paul Moore
2024-12-04  0:00     ` Shuah Khan
2024-12-04  3:45       ` Paul Moore

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=52cc8e51-9e85-465b-8ee3-63a7a0a42951@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=av2082000@gmail.com \
    --cc=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=ricardo@marliere.net \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox