From: Amit <av2082000@gmail.com>
To: Shuah Khan <skhan@linuxfoundation.org>,
Casey Schaufler <casey@schaufler-ca.com>,
paul@paul-moore.com, jmorris@namei.org, serge@hallyn.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
Subject: Re: [PATCH] selftests: lsm: Refactor `flags_overset_lsm_set_self_attr` test
Date: Sat, 16 Nov 2024 20:55:32 +0530 [thread overview]
Message-ID: <4a72bea1-da58-438f-b03e-e79bd4011f64@gmail.com> (raw)
In-Reply-To: <196eaffe-c90b-4f44-a748-b786b46fd506@linuxfoundation.org>
On 14/11/24 10:38 pm, Shuah Khan wrote:
> On 11/14/24 09:55, Casey Schaufler wrote:
>>
>> On 11/14/2024 8:25 AM, Shuah Khan wrote:
>>> 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.
V2: https://lore.kernel.org/all/20241116152136.10635-1-av2082000@gmail.com/
>>> >>> How did you find this problem? Do include the details on how
>>> in the change log.
While exploring the kselftest framework. I found the this problem.
>>>
>>>> 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?
>>
>> In the realm of linux security modules ctx is short for "context".
>> I used tctx here because I was lazy. It would be much better to
>> drop tctx, even if it means a tiny bit more change.
>>
>
> Makes sense.
>
> Amit, you can ignore this comment about tctx and ctx. Please do fix
> others about the change log and short log.
>
> thanks,
> -- Shuah
>
--
Thanks,
Amit V.
next prev parent reply other threads:[~2024-11-16 15: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
2024-11-14 16:55 ` Casey Schaufler
2024-11-14 17:08 ` Shuah Khan
2024-11-16 15:25 ` Amit [this message]
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=4a72bea1-da58-438f-b03e-e79bd4011f64@gmail.com \
--to=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 \
--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.