Re: [LTP] [PATCH] safe_macros: Fix confusing safe_read() failure output

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

> Hi!
> > > In the case that we read() less bytes than expected in the strict mode
> > > we used the same tst_brk() as for the case when read() fails. However
> > > for short reads the errno is in an udefined state and we possibly end up
> > > with confusing TBROK message. Andrea reported EACESS ernno in the TBROK
> > nit: s/ernno/errno/

> > > message on a short read() while developing tests.

> > Good catch!

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> Pushed, thanks.

> > FYI safe_write() also has TERRNO in len_strict.

> > Kind regards,
> > Petr

> > +++ lib/safe_macros.c
> > @@ -554,7 +554,7 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),

> >  		if (len_strict == SAFE_WRITE_ALL) {
> >  			if ((size_t)rval != nbyte)
> > -				tst_brkm_(file, lineno, TBROK | TERRNO,
> > +				tst_brkm_(file, lineno, TBROK,
> >  					cleanup_fn, "short write(%d,%p,%zu) "
> >  					"return value %zd",
> >  					fildes, buf, nbyte, rval);

> I guess that we are missing check for invalid return value from write()
> as well and return in the rval == -1 branch, we probably need to add:

> diff --git a/lib/safe_macros.c b/lib/safe_macros.c
> index b224a5861..3d3e7c693 100644
> --- a/lib/safe_macros.c
> +++ b/lib/safe_macros.c
> @@ -551,6 +551,14 @@ ssize_t safe_write(const char *file, const int lineno, void (cleanup_fn) (void),
>                         tst_brkm_(file, lineno, TBROK | TERRNO,
>                                 cleanup_fn, "write(%d,%p,%zu) failed",
>                                 fildes, buf, nbyte);
> +                       return rval;
> +               }
> +
> +               if (rval < 0) {
> +                       tst_brkm_(file, lineno, TBROK, cleanup_fn,
> +                                 "invalid write() return value %zi",
> +                                 rval);
> +                       return rval;
>                 }

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Please merge it directly (ideally with link to this discussion on lore).

Kind regards,
Petr

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