Re: [LTP] fs/acl: Add ACL_USER_OBJ permissions test

[linuxtestproject.agent@gmail.com]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp