linux-user/qemu.h | 1 + linux-user/elfload.c | 12 ++- linux-user/main.c | 3 +- linux-user/syscall.c | 236 ++++++++++++++++++++++++++++++++++++++++++--------- 4 files changed, 208 insertions(+), 44 deletions(-)
From: Richard Henderson <rth@twiddle.net>
If the interp_prefix is a complete chroot, it may have a *lot* of files.
Setting up the cache for this is quite expensive. Instead, use the *at
versions of various syscalls to attempt the operation in the prefix.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
Changes since v1 (Nov 2016):
* Require interp_dirfd set before trying the *at path.
r~
---
linux-user/qemu.h | 1 +
linux-user/elfload.c | 12 ++-
linux-user/main.c | 3 +-
linux-user/syscall.c | 236 ++++++++++++++++++++++++++++++++++++++++++---------
4 files changed, 208 insertions(+), 44 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 4edd7d0c08..d04924fd2e 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -437,6 +437,7 @@ void mmap_fork_start(void);
void mmap_fork_end(int child);
/* main.c */
+extern int interp_dirfd;
extern unsigned long guest_stack_size;
/* user access */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 20f3d8c2c3..5d3f7aa11d 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2203,7 +2203,17 @@ static void load_elf_interp(const char *filename, struct image_info *info,
{
int fd, retval;
- fd = open(path(filename), O_RDONLY);
+ switch (interp_dirfd > 0 && filename[0] == '/') {
+ case true:
+ fd = openat(interp_dirfd, filename + 1, O_RDONLY);
+ if (fd >= 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ fd = open(filename, O_RDONLY);
+ break;
+ }
if (fd < 0) {
goto exit_perror;
}
diff --git a/linux-user/main.c b/linux-user/main.c
index 6286661bd3..06edc2c6bc 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -99,6 +99,7 @@ unsigned long reserved_va;
static void usage(int exitcode);
static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
+int interp_dirfd;
const char *qemu_uname_release;
/* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
@@ -4297,7 +4298,7 @@ int main(int argc, char **argv, char **envp)
memset(&bprm, 0, sizeof (bprm));
/* Scan interp_prefix dir for replacement files. */
- init_paths(interp_prefix);
+ interp_dirfd = open(interp_prefix, O_CLOEXEC | O_DIRECTORY | O_PATH);
init_qemu_uname_release();
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11c9116c4a..dc4387e3e6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7228,7 +7228,18 @@ static abi_long do_name_to_handle_at(abi_long dirfd, abi_long pathname,
fh = g_malloc0(total_size);
fh->handle_bytes = size;
- ret = get_errno(name_to_handle_at(dirfd, path(name), fh, &mid, flags));
+ switch (interp_dirfd > 0 && name[0] == '/') {
+ case true:
+ ret = name_to_handle_at(interp_dirfd, name + 1, fh, &mid, flags);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = name_to_handle_at(dirfd, name, fh, &mid, flags);
+ break;
+ }
+ ret = get_errno(ret);
unlock_user(name, pathname, 0);
/* man name_to_handle_at(2):
@@ -7604,6 +7615,7 @@ static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags,
#endif
{ NULL, NULL, NULL }
};
+ int ret;
if (is_proc_myself(pathname, "exe")) {
int execfd = qemu_getauxval(AT_EXECFD);
@@ -7643,7 +7655,18 @@ static int do_openat(void *cpu_env, int dirfd, const char *pathname, int flags,
return fd;
}
- return safe_openat(dirfd, path(pathname), flags, mode);
+ switch (interp_dirfd > 0 && pathname[0] == '/') {
+ case true:
+ ret = safe_openat(interp_dirfd, pathname + 1, flags, mode);
+ if (ret >= 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = safe_openat(dirfd, pathname, flags, mode);
+ break;
+ }
+ return ret;
}
#define TIMER_MAGIC 0x0caf0000
@@ -7729,6 +7752,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
struct stat st;
struct statfs stfs;
void *p;
+ char *fn;
#if defined(DEBUG_ERESTARTSYS)
/* Debug-only code for exercising the syscall-restart code paths
@@ -8260,10 +8284,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
} else {
tvp = NULL;
}
- if (!(p = lock_user_string(arg2)))
+ if (!(fn = lock_user_string(arg2)))
goto efault;
- ret = get_errno(futimesat(arg1, path(p), tvp));
- unlock_user(p, arg2, 0);
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = futimesat(interp_dirfd, fn + 1, tvp);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = futimesat(arg1, fn, tvp);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg2, 0);
}
break;
#endif
@@ -8277,18 +8312,42 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
#endif
#ifdef TARGET_NR_access
case TARGET_NR_access:
- if (!(p = lock_user_string(arg1)))
+ if (!(fn = lock_user_string(arg1))) {
goto efault;
- ret = get_errno(access(path(p), arg2));
- unlock_user(p, arg1, 0);
+ }
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = faccessat(interp_dirfd, fn + 1, arg2, 0);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = access(fn, arg2);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg1, 0);
break;
#endif
#if defined(TARGET_NR_faccessat) && defined(__NR_faccessat)
case TARGET_NR_faccessat:
- if (!(p = lock_user_string(arg2)))
+ if (!(fn = lock_user_string(arg2))) {
goto efault;
- ret = get_errno(faccessat(arg1, p, arg3, 0));
- unlock_user(p, arg2, 0);
+ }
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = faccessat(interp_dirfd, fn + 1, arg3, 0);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = faccessat(arg1, fn, arg3, 0);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg2, 0);
break;
#endif
#ifdef TARGET_NR_nice /* not on alpha */
@@ -8415,7 +8474,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
} else {
if (!(p = lock_user_string(arg1)))
goto efault;
- ret = get_errno(acct(path(p)));
+ ret = get_errno(acct(p));
unlock_user(p, arg1, 0);
}
break;
@@ -9181,14 +9240,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
case TARGET_NR_readlink:
{
void *p2;
- p = lock_user_string(arg1);
+ fn = lock_user_string(arg1);
p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0);
- if (!p || !p2) {
+ if (!fn || !p2) {
ret = -TARGET_EFAULT;
} else if (!arg3) {
/* Short circuit this for the magic exe check. */
ret = -TARGET_EINVAL;
- } else if (is_proc_myself((const char *)p, "exe")) {
+ } else if (is_proc_myself(fn, "exe")) {
char real[PATH_MAX], *temp;
temp = realpath(exec_path, real);
/* Return value is # of bytes that we wrote to the buffer. */
@@ -9202,10 +9261,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
memcpy(p2, real, ret);
}
} else {
- ret = get_errno(readlink(path(p), p2, arg3));
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = readlinkat(interp_dirfd, fn + 1, p2, arg3);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = readlink(fn, p2, arg3);
+ break;
+ }
+ ret = get_errno(ret);
}
unlock_user(p2, arg2, ret);
- unlock_user(p, arg1, 0);
+ unlock_user(fn, arg1, 0);
}
break;
#endif
@@ -9213,20 +9283,31 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
case TARGET_NR_readlinkat:
{
void *p2;
- p = lock_user_string(arg2);
+ fn = lock_user_string(arg2);
p2 = lock_user(VERIFY_WRITE, arg3, arg4, 0);
- if (!p || !p2) {
+ if (!fn || !p2) {
ret = -TARGET_EFAULT;
- } else if (is_proc_myself((const char *)p, "exe")) {
+ } else if (is_proc_myself(fn, "exe")) {
char real[PATH_MAX], *temp;
temp = realpath(exec_path, real);
ret = temp == NULL ? get_errno(-1) : strlen(real) ;
snprintf((char *)p2, arg4, "%s", real);
} else {
- ret = get_errno(readlinkat(arg1, path(p), p2, arg4));
+ switch (interp_dirfd && fn[0] == '/') {
+ case true:
+ ret = readlinkat(interp_dirfd, fn + 1, p2, arg4);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = readlinkat(arg1, fn, p2, arg4);
+ break;
+ }
+ ret = get_errno(ret);
}
unlock_user(p2, arg3, ret);
- unlock_user(p, arg2, 0);
+ unlock_user(fn, arg2, 0);
}
break;
#endif
@@ -9395,7 +9476,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
case TARGET_NR_statfs:
if (!(p = lock_user_string(arg1)))
goto efault;
- ret = get_errno(statfs(path(p), &stfs));
+ ret = get_errno(statfs(p, &stfs));
unlock_user(p, arg1, 0);
convert_statfs:
if (!is_error(ret)) {
@@ -9425,7 +9506,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
case TARGET_NR_statfs64:
if (!(p = lock_user_string(arg1)))
goto efault;
- ret = get_errno(statfs(path(p), &stfs));
+ ret = get_errno(statfs(p, &stfs));
unlock_user(p, arg1, 0);
convert_statfs64:
if (!is_error(ret)) {
@@ -9654,18 +9735,42 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
break;
#ifdef TARGET_NR_stat
case TARGET_NR_stat:
- if (!(p = lock_user_string(arg1)))
+ if (!(fn = lock_user_string(arg1))) {
goto efault;
- ret = get_errno(stat(path(p), &st));
- unlock_user(p, arg1, 0);
+ }
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = fstatat(interp_dirfd, fn + 1, &st, 0);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = stat(fn, &st);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg1, 0);
goto do_stat;
#endif
#ifdef TARGET_NR_lstat
case TARGET_NR_lstat:
- if (!(p = lock_user_string(arg1)))
+ if (!(fn = lock_user_string(arg1))) {
goto efault;
- ret = get_errno(lstat(path(p), &st));
- unlock_user(p, arg1, 0);
+ }
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = fstatat(interp_dirfd, fn + 1, &st, AT_SYMLINK_NOFOLLOW);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = lstat(fn, &st);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg1, 0);
goto do_stat;
#endif
case TARGET_NR_fstat:
@@ -10745,20 +10850,44 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
#endif
#ifdef TARGET_NR_stat64
case TARGET_NR_stat64:
- if (!(p = lock_user_string(arg1)))
+ if (!(fn = lock_user_string(arg1))) {
goto efault;
- ret = get_errno(stat(path(p), &st));
- unlock_user(p, arg1, 0);
+ }
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = fstatat(interp_dirfd, fn + 1, &st, 0);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = stat(fn, &st);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg1, 0);
if (!is_error(ret))
ret = host_to_target_stat64(cpu_env, arg2, &st);
break;
#endif
#ifdef TARGET_NR_lstat64
case TARGET_NR_lstat64:
- if (!(p = lock_user_string(arg1)))
+ if (!(fn = lock_user_string(arg1))) {
goto efault;
- ret = get_errno(lstat(path(p), &st));
- unlock_user(p, arg1, 0);
+ }
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = fstatat(interp_dirfd, fn + 1, &st, AT_SYMLINK_NOFOLLOW);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = lstat(fn, &st);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg1, 0);
if (!is_error(ret))
ret = host_to_target_stat64(cpu_env, arg2, &st);
break;
@@ -10777,9 +10906,21 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
#ifdef TARGET_NR_newfstatat
case TARGET_NR_newfstatat:
#endif
- if (!(p = lock_user_string(arg2)))
+ if (!(fn = lock_user_string(arg2)))
goto efault;
- ret = get_errno(fstatat(arg1, path(p), &st, arg4));
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = fstatat(interp_dirfd, fn + 1, &st, arg4);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = fstatat(arg1, fn, &st, arg4);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg2, 0);
if (!is_error(ret))
ret = host_to_target_stat64(cpu_env, arg3, &st);
break;
@@ -11776,12 +11917,23 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
if (!arg2)
ret = get_errno(sys_utimensat(arg1, NULL, tsp, arg4));
else {
- if (!(p = lock_user_string(arg2))) {
+ if (!(fn = lock_user_string(arg2))) {
ret = -TARGET_EFAULT;
goto fail;
}
- ret = get_errno(sys_utimensat(arg1, path(p), tsp, arg4));
- unlock_user(p, arg2, 0);
+ switch (interp_dirfd > 0 && fn[0] == '/') {
+ case true:
+ ret = sys_utimensat(interp_dirfd, fn + 1, tsp, arg4);
+ if (ret == 0 || errno != ENOENT) {
+ break;
+ }
+ /* fallthru */
+ default:
+ ret = sys_utimensat(arg1, fn, tsp, arg4);
+ break;
+ }
+ ret = get_errno(ret);
+ unlock_user(fn, arg2, 0);
}
}
break;
@@ -11811,7 +11963,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
#if defined(TARGET_NR_inotify_add_watch) && defined(__NR_inotify_add_watch)
case TARGET_NR_inotify_add_watch:
p = lock_user_string(arg2);
- ret = get_errno(sys_inotify_add_watch(arg1, path(p), arg3));
+ ret = get_errno(sys_inotify_add_watch(arg1, p, arg3));
unlock_user(p, arg2, 0);
break;
#endif
--
2.13.6
Hi, This series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [PATCH v2] linux-user: Use *at functions instead of caching interp_prefix contents Type: series Message-id: 20171205044844.3597-1-richard.henderson@linaro.org === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu t [tag update] patchew/1510087611-1851-1-git-send-email-cota@braap.org -> patchew/1510087611-1851-1-git-send-email-cota@braap.org t [tag update] patchew/1512154296-5652-1-git-send-email-peter.maydell@linaro.org -> patchew/1512154296-5652-1-git-send-email-peter.maydell@linaro.org * [new tag] patchew/20171205044844.3597-1-richard.henderson@linaro.org -> patchew/20171205044844.3597-1-richard.henderson@linaro.org Switched to a new branch 'test' 88b8681d57 linux-user: Use *at functions instead of caching interp_prefix contents === OUTPUT BEGIN === Checking PATCH 1/1: linux-user: Use *at functions instead of caching interp_prefix contents... ERROR: do not use assignment in if condition #135: FILE: linux-user/syscall.c:8287: + if (!(fn = lock_user_string(arg2))) ERROR: braces {} are necessary for all arms of this statement #135: FILE: linux-user/syscall.c:8287: + if (!(fn = lock_user_string(arg2))) [...] ERROR: do not use assignment in if condition #160: FILE: linux-user/syscall.c:8315: + if (!(fn = lock_user_string(arg1))) { ERROR: do not use assignment in if condition #183: FILE: linux-user/syscall.c:8335: + if (!(fn = lock_user_string(arg2))) { ERROR: do not use assignment in if condition #315: FILE: linux-user/syscall.c:9738: + if (!(fn = lock_user_string(arg1))) { ERROR: do not use assignment in if condition #338: FILE: linux-user/syscall.c:9758: + if (!(fn = lock_user_string(arg1))) { ERROR: do not use assignment in if condition #364: FILE: linux-user/syscall.c:10853: + if (!(fn = lock_user_string(arg1))) { ERROR: do not use assignment in if condition #389: FILE: linux-user/syscall.c:10875: + if (!(fn = lock_user_string(arg1))) { ERROR: do not use assignment in if condition #415: FILE: linux-user/syscall.c:10909: + if (!(fn = lock_user_string(arg2))) ERROR: braces {} are necessary for all arms of this statement #415: FILE: linux-user/syscall.c:10909: + if (!(fn = lock_user_string(arg2))) [...] ERROR: do not use assignment in if condition #439: FILE: linux-user/syscall.c:11920: + if (!(fn = lock_user_string(arg2))) { total: 11 errors, 0 warnings, 419 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@freelists.org
On 5 December 2017 at 04:48, Richard Henderson <richard.henderson@linaro.org> wrote: > From: Richard Henderson <rth@twiddle.net> > > If the interp_prefix is a complete chroot, it may have a *lot* of files. > Setting up the cache for this is quite expensive. Instead, use the *at > versions of various syscalls to attempt the operation in the prefix. > > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > > Changes since v1 (Nov 2016): > * Require interp_dirfd set before trying the *at path. > > > r~ > --- > linux-user/qemu.h | 1 + > linux-user/elfload.c | 12 ++- > linux-user/main.c | 3 +- > linux-user/syscall.c | 236 ++++++++++++++++++++++++++++++++++++++++++--------- > 4 files changed, 208 insertions(+), 44 deletions(-) > > diff --git a/linux-user/qemu.h b/linux-user/qemu.h > index 4edd7d0c08..d04924fd2e 100644 > --- a/linux-user/qemu.h > +++ b/linux-user/qemu.h > @@ -437,6 +437,7 @@ void mmap_fork_start(void); > void mmap_fork_end(int child); > > /* main.c */ > +extern int interp_dirfd; > extern unsigned long guest_stack_size; > > /* user access */ > diff --git a/linux-user/elfload.c b/linux-user/elfload.c > index 20f3d8c2c3..5d3f7aa11d 100644 > --- a/linux-user/elfload.c > +++ b/linux-user/elfload.c > @@ -2203,7 +2203,17 @@ static void load_elf_interp(const char *filename, struct image_info *info, > { > int fd, retval; > > - fd = open(path(filename), O_RDONLY); > + switch (interp_dirfd > 0 && filename[0] == '/') { > + case true: gcc doesn't like this pattern: /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c: In function ‘do_name_to_handle_at’: /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c:7231:13: error: switch condition has boolean value [-Werror=switch-bool] switch (interp_dirfd > 0 && name[0] == '/') { ^ It seems like an odd way to write an if(). thanks -- PMM
On 12/07/2017 05:05 AM, Peter Maydell wrote: > On 5 December 2017 at 04:48, Richard Henderson > <richard.henderson@linaro.org> wrote: >> From: Richard Henderson <rth@twiddle.net> >> >> If the interp_prefix is a complete chroot, it may have a *lot* of files. >> Setting up the cache for this is quite expensive. Instead, use the *at >> versions of various syscalls to attempt the operation in the prefix. >> >> Signed-off-by: Richard Henderson <rth@twiddle.net> >> --- >> >> Changes since v1 (Nov 2016): >> * Require interp_dirfd set before trying the *at path. >> >> >> r~ >> --- >> linux-user/qemu.h | 1 + >> linux-user/elfload.c | 12 ++- >> linux-user/main.c | 3 +- >> linux-user/syscall.c | 236 ++++++++++++++++++++++++++++++++++++++++++--------- >> 4 files changed, 208 insertions(+), 44 deletions(-) >> >> diff --git a/linux-user/qemu.h b/linux-user/qemu.h >> index 4edd7d0c08..d04924fd2e 100644 >> --- a/linux-user/qemu.h >> +++ b/linux-user/qemu.h >> @@ -437,6 +437,7 @@ void mmap_fork_start(void); >> void mmap_fork_end(int child); >> >> /* main.c */ >> +extern int interp_dirfd; >> extern unsigned long guest_stack_size; >> >> /* user access */ >> diff --git a/linux-user/elfload.c b/linux-user/elfload.c >> index 20f3d8c2c3..5d3f7aa11d 100644 >> --- a/linux-user/elfload.c >> +++ b/linux-user/elfload.c >> @@ -2203,7 +2203,17 @@ static void load_elf_interp(const char *filename, struct image_info *info, >> { >> int fd, retval; >> >> - fd = open(path(filename), O_RDONLY); >> + switch (interp_dirfd > 0 && filename[0] == '/') { >> + case true: > > gcc doesn't like this pattern: > > /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c: In > function ‘do_name_to_handle_at’: > /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c:7231:13: > error: switch condition has boolean value [-Werror=switch-bool] > switch (interp_dirfd > 0 && name[0] == '/') { > ^ > > It seems like an odd way to write an if(). Huh. My gcc likes it just fine. What version are you using? It's an IF with a /* fallthru */, so there are very few less ugly ways to write it... r~
On 7 December 2017 at 14:43, Richard Henderson <rth@twiddle.net> wrote: > On 12/07/2017 05:05 AM, Peter Maydell wrote: >> gcc doesn't like this pattern: >> >> /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c: In >> function ‘do_name_to_handle_at’: >> /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c:7231:13: >> error: switch condition has boolean value [-Werror=switch-bool] >> switch (interp_dirfd > 0 && name[0] == '/') { >> ^ >> >> It seems like an odd way to write an if(). > > Huh. My gcc likes it just fine. What version are you using? > > It's an IF with a /* fallthru */, so there are very few > less ugly ways to write it... gcc (Ubuntu 5.4.0-6ubuntu1~16.04.5) 5.4.0 20160609 (for x86-64). thanks -- PMM
On 12/07/2017 08:43 AM, Richard Henderson wrote: >>> - fd = open(path(filename), O_RDONLY); >>> + switch (interp_dirfd > 0 && filename[0] == '/') { >>> + case true: >> >> gcc doesn't like this pattern: >> >> /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c: In >> function ‘do_name_to_handle_at’: >> /home/petmay01/linaro/qemu-from-laptop/qemu/linux-user/syscall.c:7231:13: >> error: switch condition has boolean value [-Werror=switch-bool] >> switch (interp_dirfd > 0 && name[0] == '/') { >> ^ >> >> It seems like an odd way to write an if(). > > Huh. My gcc likes it just fine. What version are you using? > > It's an IF with a /* fallthru */, so there are very few > less ugly ways to write it... Maybe: while (1) { if (interp_dirfd > 0 && filename[0] == '/') { fd = openat(interp_dirfd, filename + 1, O_RDONLY); if (fd >= 0 || errno != ENOENT) { break; } } fd = open(filename, O_RDONLY); break; } -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 12/07/2017 09:06 AM, Eric Blake wrote: >> It's an IF with a /* fallthru */, so there are very few >> less ugly ways to write it... > > Maybe: > > while (1) { > if (interp_dirfd > 0 && filename[0] == '/') { > fd = openat(interp_dirfd, filename + 1, O_RDONLY); > if (fd >= 0 || errno != ENOENT) { > break; > } > } > fd = open(filename, O_RDONLY); > break; > } Nice. I'd been pondering if (interp_dirfd < 0 || filename[0] != '/' || (fd = openat(interp_dirfd, filename + 1, O_RDONLY), fd < 0 && errno == ENOENT)) { fd = open(filename, O_RDONLY); } r~
© 2016 - 2024 Red Hat, Inc.