Re: [LTP] Add safe macros for stream testing suite

[acervesato@git]

Hi Andrea,

our agent completed the review of the patch.

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.

--- Reply to [PATCH 1/6] ---

On 2026-03-04, Andrea Cervesato wrote:
> Add safe macros for stream testing suite

> +size_t safe_fread(const char *file, const int lineno,
> +	void *ptr, size_t size, size_t n, FILE *stream)
> +{
> +	size_t ret;
> +
> +	ret = fread(ptr, size, n, stream);
> +	if (ret != n) {
> +		tst_brkm_(file, lineno, TBROK, NULL,
> +			"fread(%p, %lu, %lu, %p) read %lu bytes",
> +			ptr, size, n, stream, ret);

%lu is wrong for size_t on ILP32; use %zu. Also, ret is an item count,
not bytes — the message is misleading. Same issue in safe_fwrite.

Use TBROK | TERRNO here and in safe_fwrite; errno is set on actual I/O
errors, not just EOF.

> +FILE *safe_freopen(const char *file, const int lineno,
> +	       const char *path, const char *mode, FILE *stream)

Continuation-line alignment is inconsistent with the declaration in
safe_stdio_fn.h. Same applies to safe_fseek, safe_ftell. Align to the
opening parenthesis using tabs+spaces as in the header.

--- Reply to [PATCH 2/6] ---

On 2026-03-04, Andrea Cervesato wrote:
> fs: rewrite stream01 test using new API

Commit body is missing. Explain why the conversion is done (e.g. "Old
API is deprecated; convert to new tst_test.h-based API."). Same applies
to patches 3/6–6/6.

> +static void read_file(const char *file, const char *str, size_t n)
> +{
> +	char buf[n];
> +	...
> +	len = SAFE_FREAD(buf, n, n, stream);

fread(buf, n, n) attempts to read n*n bytes into a n-byte VLA — buffer
overflow. Change to SAFE_FREAD(buf, 1, n, stream).

> +	SAFE_FWRITE(buff_file1, strlen(buff_file1), 3, stream);

fwrite("abc", 3, 3) reads 9 bytes from a 3-byte literal — out-of-bounds
read. Change to SAFE_FWRITE(buff_file1, 1, strlen(buff_file1), stream).

--- Reply to [PATCH 3/6] ---

On 2026-03-04, Andrea Cervesato wrote:
> fs: rewrite stream02 test using new API

> +		stream = SAFE_FOPEN(FILENAME, modes[i]);
> +		if (!stream) {
> +			tst_res(TFAIL, "fopen(%s) returned NULL pointer", modes[i]);
> +			return;
> +		}

SAFE_FOPEN never returns NULL — it calls tst_brk(TBROK) on failure.
Remove the dead null check.

--- Reply to [PATCH 4/6] ---

On 2026-03-04, Andrea Cervesato wrote:
> fs: rewrite stream03 test using new API

> +	rewind(stream);
> +	TST_EXP_EQ_LI(SAFE_FTELL(stream), SEEK_SET);

SEEK_SET is a seek-direction constant (value 0), not a file position.
Use 0 as the expected value to avoid confusion.

Regards,
LTP AI Reviewer

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