[LTP] fs/acl: Add ACL_USER_OBJ permissions test
linuxtestproject.agent@gmail.com
linuxtestproject.agent@gmail.com
Wed Jun 3 11:43:17 CEST 2026
Hi Sachin,
Thank you for the patches. This is a welcome addition — replacing the old
unmaintained bash script with a proper set of modular C tests with full LTP
framework integration. Below are my review comments.
---
[PATCH 1/8] fs/acl: Add ACL_USER_OBJ permissions test
acl_lib.h: Copyright header
The .c and .h files all carry:
* Copyright (c) IBM, 2026
The correct LTP convention is:
* Copyright (c) 2026 IBM
(year first, then author, no comma). Same applies to all .c/.h files
in the series.
acl_user_obj01.c: The test logic is correct.
SAFE_CHMOD(TESTDIR, 0500) followed by try_create_as() expecting EACCES
is sound. Setting a minimal ACL (USER_OBJ, GROUP_OBJ, OTHER) via
acl_set_file() updates the inode mode bits, so USER_OBJ=rwx makes the
subsequent creation succeed. No issues here.
---
[PATCH 3/8] fs/acl: Add symlink ACL operations test
acl_link01.c: Test does not verify what it claims
The commit message says:
"acl_set_file() on a symlink sets ACL on the target file"
But the test only checks the return value of acl_set_file() and
acl_get_file() through the symlink — it never verifies that the ACL
was actually set on TESTFILE (the symlink target).
The retrieved ACL is read through the symlink and then immediately
freed without inspecting its contents or comparing it with what was
set. So the claim that "ACL operations follow symlinks rather than
operating on the link itself" is not actually proven.
To properly verify this, also call acl_get_file(TESTFILE, ...) and
confirm the expected entries are present, or at minimum compare the
ACL retrieved via the symlink against the ACL retrieved via the
direct file path.
---
[PATCH 7/8] fs/acl: Add extended attributes test
Commit message body contains file-change bullets:
The last two bullet points in the commit body are:
- Update runtest/fs to include xattr_test01
- Update .gitignore for new test binary
These describe file modifications rather than the test's purpose or
rationale. The diff already shows what files were modified. Remove
these bullets from the commit message body; keep only the description
of what the test validates.
---
Pre-existing issues (informational, not blocking):
None identified.
---
Verdict: Needs revision
Regards,
LTP AI Reviewer
More information about the ltp
mailing list