[LTP] syscalls/xattr: Add runtime probe for socket xattr backports
linuxtestproject.agent@gmail.com
linuxtestproject.agent@gmail.com
Mon Jun 1 15:35:18 CEST 2026
Hi Darren,
Thank you for this patch. The approach is sound: a runtime probe for
backported socket xattr support is the right way to avoid false
failures on downstream kernels. However there are a few issues in
fsetxattr02.c that need fixing before this can be applied.
Verdict: Needs revision
---
[1] fsetxattr02.c — variable declared after executable statement
The new `int socket_idx = -1;` declaration appears after the
executable statement `socket_xattr_supported = tst_kvercmp(7, 1, 0)
>= 0;`. The existing setup() declares all its local variables at the
top of the function (`size_t i = 0;` etc.). This new variable must
follow the same convention.
Move `int socket_idx = -1;` to the top of setup(), alongside the
existing declarations:
static void setup(void)
{
size_t i = 0;
int socket_idx = -1; /* add here */
struct sockaddr_un sun;
...
[2] fsetxattr02.c — indentation uses spaces instead of tabs
The `int socket_idx = -1;` line in the patch uses 4 spaces for
indentation instead of a tab character:
- int socket_idx = -1; /* 4 spaces — wrong */
+ int socket_idx = -1; /* tab — correct */
The rest of the function uses tabs throughout.
[3] fsetxattr02.c — comment doesn't reference the upstream commit
The comment added to fsetxattr02.c is vague compared to the one added
to setxattr02.c, which names the upstream commit and its title:
setxattr02.c (new):
/*
* Commit dc0876b9846d "xattr: support extended attributes on sockets"
* merged in 7.1.0 mainline, but might be backported.
* Try to set xattr on socket to detect support.
*/
fsetxattr02.c (new):
/*
* Probe for backports on older kernels.
* If the version is < 7.1.0 but fsetxattr() succeeds, we assume the patch
* has been backported.
*/
These describe the same feature. The fsetxattr02 comment should
include the commit reference for consistency. Use the same wording as
in setxattr02.c.
[4] fsetxattr02.c — inline comment is unnecessary
/* Clean up the probed xattr to avoid interfering with the actual test */
SAFE_FREMOVEXATTR(tc[socket_idx].fd, tc[socket_idx].key);
The comment states the obvious. SAFE_FREMOVEXATTR after a successful
setxattr-probe speaks for itself. Drop it, matching the style of the
equivalent block in setxattr02.c which has no such comment.
---
Aside from those four points the logic is correct. In particular:
- setxattr02.c looks clean; no issues there.
- The probe correctly uses the already-opened socket fd in fsetxattr02,
mirroring the path-based probe in setxattr02.
- Silently ignoring non-zero probe results is correct: EPERM means no
backport, and EOPNOTSUPP is caught later in verify_fsetxattr().
- `socket_xattr_supported = true` (using bool) is consistent with the
field type (`static bool socket_xattr_supported`).
- The commit message is accurate and the Signed-off-by is present.
---
LTP AI Reviewer
More information about the ltp
mailing list