[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation
From: |
Aleksandar Markovic |
Subject: |
Re: [Qemu-devel] [PATCH v12 3/5] linux-user: Add support for translation of statx() syscall |
Date: |
Thu, 27 Jun 2019 13:18:30 +0000 |
> From: Laurent Vivier <address@hidden>
>
> > @@ -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
[Qemu-devel] [PATCH v12 4/5] linux-user: Add support for strace for statx() syscall, Aleksandar Markovic, 2019/06/19
Re: [Qemu-devel] [PATCH v12 0/5], no-reply, 2019/06/19
Re: [Qemu-devel] [PATCH v12 0/5], no-reply, 2019/06/19