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

Cyril Hrubis
2022-11-24T11:58:23Z

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.

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