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.