Re: [PATCH] selftests: lsm: Refactor `flags_overset_lsm_set_self_attr` test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/26/24 20:38, Paul Moore wrote:
On Thu, Nov 14, 2024 at 11:25 AM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> 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.

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

Signed-off-by: Amit Vadhavana <av2082000@xxxxxxxxx>
---
   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@xxxxxxxxxxxxxxxxxxx>

Paul, James,

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

Amit has already posted a v2 with the requested changes, but I wanted
to get back to you on this even if this patch is outdated ... Shuah,
what is your preference for things like this?  My general policy is
that patches only affecting one subsystem tree should be taken by the
associated subsystem to minimize merge headaches and other ugliness,
however, the kselftest is an interesting subsystem in that it relies
so heavily on others that I'm not sure my policy makes as much sense
here :)


kselftest patches usually go through subsystem trees because of the
merge problems you mentioned. I take them through kselftest tree
if subsystem maintainers want me to. Some do and I pick them up.

I pick up patches if I don't see response from subsystem maintainers.

thanks,
-- Shuah




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux