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

[linuxtestproject.agent@gmail.com]

Hi Sachin,

On Mon, 8 Jun 2026 14:51:53 +0530, Sachin Sant wrote:
> fs/acl: Add ACL_USER_OBJ permission test

--- [PATCH 1/8] ---

> +#include <sys/xattr.h>

acl_lib.h includes <sys/xattr.h> unconditionally. LTP's configure.ac
checks for this header and defines HAVE_SYS_XATTR_H. Other xattr tests
(e.g. setxattr01, setxattr02, setxattr03) guard their code with
#ifdef HAVE_SYS_XATTR_H.

Each .c test file should wrap all test code in a file-level guard with
TST_TEST_TCONF in the #else branch, e.g.:

  #include "acl_lib.h"

  #ifdef HAVE_SYS_XATTR_H

  /* ... all test code ... */

  static struct tst_test test = { ... };

  #else
      TST_TEST_TCONF("sys/xattr.h is not available");
  #endif

Without this, the tests will fail to compile on systems where
sys/xattr.h is not available.

--- [PATCH 6/8] ---

> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.mount_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.forks_child = 1,

acl_link01 never forks a child process. It does not call SAFE_FORK(),
try_create_as(), or create_file_as(). Should .forks_child be dropped?

--- [PATCH 7/8] ---

> +	TST_EXP_PASS_SILENT(setxattr(TESTDIR, XATTR_TEST_DIR_NAME,
> +				     XATTR_TEST_DIR_VALUE,
> +				     XATTR_TEST_DIR_SIZE, 0));
> +	if (!TST_PASS) {
> +		if (TST_ERR == EOPNOTSUPP) {
> +			tst_res(TCONF, "Extended attributes not supported");
> +			return;
> +		}
> +		tst_res(TFAIL, "setxattr on directory failed");
> +		return;
> +	}

TST_EXP_PASS_SILENT already reports TFAIL on failure. Then the code
reports TFAIL again (or TCONF for EOPNOTSUPP), resulting in
double-reporting. For the EOPNOTSUPP case the output will contain a
contradictory TFAIL followed by TCONF.

Since custom failure handling is needed here (EOPNOTSUPP -> TCONF),
TEST() should be used instead:

  TEST(setxattr(TESTDIR, XATTR_TEST_DIR_NAME,
                XATTR_TEST_DIR_VALUE, XATTR_TEST_DIR_SIZE, 0));
  if (TST_RET == -1) {
      if (TST_ERR == EOPNOTSUPP) {
          tst_res(TCONF, "Extended attributes not supported");
          return;
      }
      tst_res(TFAIL | TTERRNO, "setxattr on directory failed");
      return;
  }

The same pattern appears for the second setxattr call in test_xattr()
and the first setxattr call in test_xattr_backup_restore().

> +	.forks_child = 1,

Same as patch 6/8: xattr_test01 never forks a child process.
Should .forks_child be dropped?

--- Applies to patches 2/8 through 7/8 ---

> + * [Algorithm]
> + *
> + * 1. Set up ACL with full permissions for the entry

The [Algorithm] section must use a bulleted list with "-" markers,
not numbered lists. This applies to all six tests that have an
[Algorithm] block (acl_mask01, acl_other01, acl_inherit01,
acl_file_ops01, acl_link01, xattr_test01).

Verdict: Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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