On 08/08/2017 01:48 PM, Philippe Mathieu-Daudé wrote: >> + fd = openat_file(dirfd, name, O_RDONLY | O_PATH, 0); > > since you use O_PATH, you can drop O_RDONLY.
Technically, POSIX says (and 'man 2 open' agrees, modulo the fact that Linux still lacks O_SEARCH) that you MUST provide one of the 5 access modes (they are O_RDONLY, O_RDWR, O_WRONLY, O_EXEC, and O_SEARCH; although POSIX allows O_EXEC and O_SEARCH to be the same bit pattern), and then bitwise-or any additional flags. So the usage here is correct. Practically, though, this code is ONLY compilable on Linux (no one else has O_PATH yet, although it is a useful Linux invention), and on Linux, O_RDONLY is 0. So behaviorally, you are correct that open(O_PATH) rather than open(O_RDONLY | O_PATH) does the same thing, even if it sets a bad example. [Well, insofar as you don't have any other bugs by omitting O_NOFOLLOW, per my other thread...] >> +++ b/hw/9pfs/9p-util.h >> @@ -43,9 +43,13 @@ static inline int openat_file(int dirfd, const char >> *name, int flags, >> } >> serrno = errno; >> - /* O_NONBLOCK was only needed to open the file. Let's drop it. */ >> - ret = fcntl(fd, F_SETFL, flags); >> - assert(!ret); >> + /* O_NONBLOCK was only needed to open the file. Let's drop it. We >> don't >> + * do that with O_PATH since it ignores O_NONBLOCK. >> + */ > > thinking about if someone uses openat_file(d, O_PATH|O_CLOEXEC, ...), > why not set the fd flags always? like: > > ret = fcntl(fd, F_SETFL, flags); > assert((flags & O_PATH) || !ret); Syscalls are expensive. It's better to avoid fcntl() up front than it is to skip failure after the fact. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
