Re: [LTP] safe_stdio: More checks for invalid function return values

[Petr Vorel <pvorel@suse.cz>]

> Hi Petr,

> --- [PATCH 1/2] ---

> On 2026-04-17, Petr Vorel wrote:
> > safe_stdio: More checks for invalid function return values

> > -	if (ret == EOF)
> > -		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p)", f);
> > +	if (ret == EOF) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL, "fflush(%p) failed", f);
> > +	} else if (!ret) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +				  "Invalid fflush(%p) return value %d", f, ret);
> > +	}

> !ret is ret == 0, which is fflush()'s success return — this breaks every
> caller on success. fflush() only returns 0 or EOF per POSIX; there is no
> invalid return value to guard against, so drop the else if branch entirely.

Yeah, this one is really invalid, thanks agent :).
I'll wait little longer for others feedback before sending v2.

> > +	} else if (ret < 0) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +				  "Invalid ftell(%p) return value %ld", f, ret);

> > +	} else if (ret < 0) {
> > +		tst_brkm_(file, lineno, TBROK | TERRNO, NULL,
> > +				  "Invalid fileno(%p) return value %d", f, ret);

> Use ret < -1 here to be consistent with safe_fseek() in the same file and
> with the policy established by patch 2/2; also fix the continuation-line
> indentation (tabs only, no mixed spaces).

I frankly expect second patch being refused. If not, I'd swap commits in v2.

> --- [PATCH 2/2] ---

> On 2026-04-17, Petr Vorel wrote:
> > lib: Be pedantic on invalid comparison check

> > diff --git a/lib/safe_stdio.c b/lib/safe_stdio.c
> > @@ -67,7 +67,7 @@ int safe_asprintf(...)
> > -	} else if (ret < 0) {
> > +	} else if (ret < -1) {
> > ...
> > @@ -154,7 +154,7 @@ int safe_fseek(...)
> > -	} else if (ret < 0) {
> > +	} else if (ret < -1) {

> This fixes asprintf and fseek but misses safe_ftell() and safe_fileno()
> from patch 1/2, which are left with ret < 0. Either fix them here or
> swap the patch order so new checks are written with ret < -1 from the start.

Good catch. I was sure I would not forget, but obviously I did.

Kind regards,
Petr

> ---
> Note:

> Our agent completed the review of the patch. The full review can be found
> in the conversation context above.

> 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