Re: [LTP] syscalls/xattr: Add runtime probe for socket xattr backports

[linuxtestproject.agent@gmail.com]

Hi Darren,

Thank you for the patch. The approach is sound: using a runtime probe in
setup() to detect a backported feature is the correct LTP pattern when a
version check alone is insufficient. One issue needs fixing before merging.

---

syscalls/xattr: Add runtime probe for socket xattr backports

Commit message: OK

> diff --git a/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c b/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c

Lines 8-26 (/*\ description comment, not changed by the patch):

  /*\
   * ...
   * - Set attribute to a UNIX domain socket, fsetxattr(2) should
   *   return -1 and set errno to EPERM on kernels < 7.1.0.
   *   On kernel 7.1.0+ (dc0876b9846d "xattr: support extended
   *   attributes on sockets"), returns 0 (success) as sockets now
   *   support user.* xattrs.
   */

The patch changes the socket test-case behavior for kernels < 7.1.0 with
the backport, but does not update this description. After the patch there
are three outcomes for the socket case:

  - kernel < 7.1.0, no backport     → EPERM
  - kernel < 7.1.0, backport present → 0 (success, detected by probe)
  - kernel >= 7.1.0                  → 0 (success)

The comment currently describes only the first and third case. Please
update it to mention the runtime probe for backported kernels. For
example:

  * - Set attribute to a UNIX domain socket, fsetxattr(2) should
  *   return -1 and set errno to EPERM on kernels < 7.1.0, unless the
  *   feature is detected at runtime (e.g. backport). On kernel 7.1.0+
  *   (dc0876b9846d "xattr: support extended attributes on sockets"),
  *   or when the backport is probed in setup(), returns 0 (success).

Note: setxattr02.c uses "EPERM/SUCCEED" in its description which is
already open-ended; no change is needed there.

> +       int socket_idx = -1;
> ...
> +               for (i = 0; i < ARRAY_SIZE(tc); i++) {
> +                       if (tc[i].issocket) {
> +                               socket_idx = i;

Nit: `socket_idx` is `int` but assigned from `size_t i`. Not a runtime
problem here (small array), but declaring it as `size_t socket_idx` with
`ARRAY_SIZE(tc)` as the "not found" sentinel would avoid the implicit
narrowing. If you keep `int`, that is also acceptable given the tiny
array size.

> diff --git a/testcases/kernel/syscalls/setxattr/setxattr02.c b/testcases/kernel/syscalls/setxattr/setxattr02.c

New probe code: OK. Clean and straightforward.

---

Verdict: Needs revision 🔴

The description comment in fsetxattr02.c must be updated to reflect the
three possible outcomes for the socket test case. Everything else is
correct.

LTP AI Reviewer

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