Re: [LTP] [PATCH v5 4/7] Hugetlb: Safe macro for posix_fadvise call

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > >  include/tst_safe_macros.h | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
> > > index 81c4b0844..4965e44d0 100644
> > > --- a/include/tst_safe_macros.h
> > > +++ b/include/tst_safe_macros.h
> > > @@ -298,6 +298,23 @@ static inline int safe_ftruncate(const char *file, const int lineno,
> > >  #define SAFE_FTRUNCATE(fd, length) \
> > >  	safe_ftruncate(__FILE__, __LINE__, (fd), (length))
> > >  
> > > +static inline int safe_posix_fadvise(const char *file, const int lineno,
> > > +                                int fd, off_t offset, off_t len, int advice)
> > > +{
> > > +	int rval;
> > > +
> > > +	rval = posix_fadvise(fd, offset, len, advice);
> > > +
> > > +	if (rval)
> > > +		tst_brk_(file, lineno, TBROK | TERRNO,
> > > +			"posix_fadvise(%d,%ld,%ld,%d) failed",
> > > +			fd, (long)offset, (long)len, advice);
> > 
> > I did a closer look at the posix_fadvise() manual and it returns an
> > error in case of a failure, so the TERRNO is wrong here since that
> > would print whatever was the last error stored in there.
> > 
> > So we either have to do errno = rval; in the if (rval) branch or use
> > tst_strerrno() to print the errno ourselves.
> > 
> > Looking at the code we have the safe_pthread.c does use tst_strerrno()
> > to print the error, so I would be inclined to do so here as well.
> > 
> 
> posix_fadvise should be inline, and so for the convention it will be part of
> tst_safe_macros.h
> tst_strerrno is defined in lib/errnos.h
> 
> there are only two such header file declared in this way. errnos.h and
> signame.h. To be able to use tst_strerrno, it will require some cleanup
> (like converting .h to .c and transfer PAIR, STRPAIR macros definition to
> header files too from tst_res.c).
> 
> As, Even looking at past commits, I am not sure why these two file are
> implememnted as .h, and not available as in general. I am currently
> thinking of errno = rval way.
> 
> Can those file be converted?

Isn't the tst_strerrno() prototype defined in the tst_test.h?

The headers in lib/ are strictly internal implementation and shouldn't
be used from tests at all. And they don't have to be included at all,
since these are included in the test library in order to provide the
implementation for these functions.

-- 
Cyril Hrubis
chrubis@suse.cz

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