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

[linuxtestproject.agent@gmail.com]

Hi Darren,

Thank you for this patch. The overall approach — using a runtime probe to
detect backported socket xattr support rather than relying solely on the
version check — is correct and useful for Android downstream kernels.

A couple of issues to fix before merging:

---

[COMMIT MESSAGE]

> was merged in v7.1.0

> strictly expect EPERM for sockets on kernels < 7.1.0.

Kernel mainline releases are tagged as "v7.1", not "v7.1.0". The sublevel
".0" is not part of the release tag. Please replace "v7.1.0" → "v7.1" and
"7.1.0" → "7.1" throughout the commit message.

---

[testcases/kernel/syscalls/fsetxattr/fsetxattr02.c]

> +		if (socket_idx != -1) {
> +			if (fsetxattr(tc[socket_idx].fd, tc[socket_idx].key,
> +				      tc[socket_idx].value, tc[socket_idx].size,
> +				      tc[socket_idx].flags) == 0) {
> +
> +				tst_res(TINFO, "Socket xattr support detected (backport)");

Blank line between the opening brace and the first statement. Linux kernel
coding style does not permit blank lines immediately after an opening '{'.
Please remove the blank line.

Also, the inline code comment (line just added in setup()) reads:

> +	 * merged in 7.1.0 mainline, but might be backported.

Same version notation issue: "7.1.0" → "7.1".

---

[testcases/kernel/syscalls/setxattr/setxattr02.c]

> +		if (setxattr(SOCK, XATTR_TEST_KEY, XATTR_TEST_VALUE,
> +			     XATTR_TEST_VALUE_SIZE, XATTR_CREATE) == 0) {
> +
> +			tst_res(TINFO, "Socket xattr support detected (backport)");

Same blank-line-after-brace issue as in fsetxattr02.c. Remove the blank
line.

Also:

> +	 * merged in 7.1.0 mainline, but might be backported.

"7.1.0" → "7.1".

---

Verdict: Needs revision

Regards,
LTP AI Reviewer

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