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

[Tarun Sahu]

On Nov 24 2022, Cyril Hrubis wrote:
> 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?


> Also LKML coding style prefers curly braces around multiline blocks, so
> I would enclose the multiline tst_brk_() into curly braces, but that is
> very minor.
> 
> > +	return rval;
> > +}
> > +#define SAFE_POSIX_FADVISE(fd, offset, len, advice) \
> > +	safe_posix_fadvise(__FILE__, __LINE__, (fd), (offset), (len), (advice))
> > +
> >  static inline int safe_truncate(const char *file, const int lineno,
> >                                  const char *path, off_t length)
> >  {
> > -- 
> > 2.31.1
> > 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp