Re: [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall

[Aleksandar Markovic <amarkovic@wavecomp.com>]

> From: Laurent Vivier <laurent@vivier.eu>
> 
> > @@ -10173,6 +10225,88 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long > arg1,
> >              ret = host_to_target_stat64(cpu_env, arg3, &st);
> >          return ret;
> >  #endif
> > +#if defined(TARGET_NR_statx)
> > +    case TARGET_NR_statx:
> > +        {
> > +            struct target_statx *target_stx;
> > +            int dirfd = arg1;
> > +            int flags = arg3;
> > +
> > +            p = lock_user_string(arg2);
> > +            if (p == NULL) {
> > +                return -TARGET_EFAULT;
> > +            }
> > +#if defined(__NR_statx)
> > +            {
> > +                /*
> > +                 * It is assumed that struct statx is architecture independent.
> > +                 */
> > +                struct target_statx host_stx;
> > +                int mask = arg4;
> > +
> > +                ret = get_errno(statx(dirfd, p, flags, mask, &host_stx));
> > +                if (!is_error(ret)) {
> > +                    if (host_to_target_statx(&host_stx, arg5) != 0) {
> > +                        unlock_user(p, arg2, 0);
> > +                        return -TARGET_EFAULT;
> > +                    }
> > +                }
> > +
> > +                if (ret != -TARGET_ENOSYS) {
> > +                    unlock_user(p, arg2, 0);
> > +                    return ret;
> > +                }
> > +            }
> > +#endif
> > +            if (*((char *)p) == 0) {
> > +                /*
> > +                 * By file descriptor
> > +                 */
> > +                if (flags & AT_EMPTY_PATH) {
> > +                    unlock_user(p, arg2, 0);
> > +                    return -TARGET_ENOENT;
> > +                }
> > +                ret = get_errno(fstat(dirfd, &st));
> > +            } else if (*((char *)p) == '/') {
> > +                /*
> > +                 * By absolute pathname
> > +                 */
> > +                ret = get_errno(stat(path(p), &st));
> > +            } else {
> > +                /*
> > +                 * By pathname relative to the current working directory
> > +                 * (if 'dirfd' is AT_FDCWD) or 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);
> 
> Could you explain why we can't use fstatat() for the two previous cases
> "(*((char *)p) == 0)" and "(*((char *)p) == '/')"?
> 

Man page on fstatat (http://man7.org/linux/man-pages/man2/stat.2.html)
says:

   AT_EMPTY_PATH (since Linux 2.6.39)
          If pathname is an empty string, operate on the file referred
          to by dirfd (which may have been obtained using the open(2)
          O_PATH flag).  In this case, dirfd can refer to any type of
          file, not just a directory, and the behavior of fstatat() is
          similar to that of fstat().  If dirfd is AT_FDCWD, the call
          operates on the current working directory.  This flag is
          Linux-specific; define _GNU_SOURCE to obtain its definition.

So it looks the branch "if (*((char *)p) == 0)" can be handled by
fstatat().

Also, the man page says:

   If pathname is absolute, then dirfd is ignored.

So, it looks the case "else if (*((char *)p) == '/')" can also be
handled by fstatat().

Very similar descriptions of the cases above can be found in
the man page for statx
(http://man7.org/linux/man-pages/man2/statx.2.html).

The whole string of if statements after "#endif" above should be now,
in my opinion:

        ret = get_errno(fstatat(dirfd, path(p), &st, flags));
        unlock_user(p, arg2, 0);

... and I will submit the patch with such code, if noone objects.

Yours,
Aleksandar Markovic

> Thanks,
> Laurent