> From: Timothy Baldwin <t.e.baldwi...@members.leeds.ac.uk>
> Sent: Monday, August 20, 2018 9:48 AM
>
> Subject: Re: [PATCH v8 81/87] linux-user: Add support for statx() syscall for
> all platforms
>
> On 13/08/18 18:53, Aleksandar Markovic wrote:
> > From: Aleksandar Rikalo <arik...@wavecomp.com>
> >
> > Implement support for translation of system call statx(). The
> > implementation includes invoking other (more mature) syscalls
> > (from the same 'stat' family) on the host side. This way,
> > problems of availability of statx() on the host side are
> > avoided.
> >
> > Signed-off-by: Aleksandar Markovic <amarko...@wavecomp.com>
> > Signed-off-by: Stefan Markovic <smarko...@wavecomp.com>
> > ---
> > linux-user/syscall.c | 121
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > linux-user/syscall_defs.h | 38 +++++++++++++++
> > 2 files changed, 158 insertions(+), 1 deletion(-)
> >
>
>
> > + if ((p == NULL) || (*((char *)p) == 0)) {
> > + /* By file descriptor */
> > + ret = get_errno(fstat(dirfd, &st));
> > + unlock_user(p, arg2, 0);
> > + } else if (*((char *)p) == '/') {
> > + /* Absolute pathname */
> > + ret = get_errno(stat(path(p), &st));
> > + unlock_user(p, arg2, 0);
> > + } else {
> > + if (dirfd == AT_FDCWD) {
> > + /* Pathname relative to the current working directory
> > */
> > + ret = get_errno(stat(path(p), &st));
> > + unlock_user(p, arg2, 0);
> > + } else {
> > + /*
> > + * Pathname relative to the directory referred to by
> > the
> > + * file descriptor dirfd
> > + */
> > + ret = get_errno(fstatat(dirfd, path(p), &st, flags));
> > + unlock_user(p, arg2, 0);
> > + }
> > + }
>
> This doesn't correctly handle the flags argument, it is ignored unless a
> relative path and directory file descriptor is provided.
Hi, Timothy.
Agreed. The patch generally needs certain improvements wrt handling original
statx() arguments. I would certianly add handling the mask argument.
> As such an implementation of lstat that uses statx will be broken.
>
> Since fstatat exists since 2.6.16 this can be reduced to a call to fstatat.
I am not sure what you meant here. I think QEMU lstat() implementation does not
use statx(). This implementation of statx() uses hosts's statx(), and, as a
fallback, host's fstat(), stat(), and fstatat().
I would like to add that I think this patch should be submitted separately, out
of this series.
Yours,
Aleksandar
