From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by passt.top (Postfix, from userid 1000) id 6C7905A061F; Wed, 30 Oct 2024 09:09:09 +0100 (CET) From: Stefano Brivio To: passt-dev@passt.top Subject: [PATCH v5 5/8] treewide: Suppress clang-tidy warning if we already use O_CLOEXEC Date: Wed, 30 Oct 2024 09:09:06 +0100 Message-ID: <20241030080909.2781504-6-sbrivio@redhat.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20241030080909.2781504-1-sbrivio@redhat.com> References: <20241030080909.2781504-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: DC3QE2BYCNT2ZXAYM7SP6BJQ2LZHBMSO X-Message-ID-Hash: DC3QE2BYCNT2ZXAYM7SP6BJQ2LZHBMSO X-MailFrom: sbrivio@passt.top X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: In pcap_init(), we should always open the packet capture file with O_CLOEXEC, even if we're not running in foreground: O_CLOEXEC means close-on-exec, not close-on-fork. In logfile_init() and pidfile_open(), the fact that we pass a third 'mode' argument to open() seems to confuse the android-cloexec-open checker in LLVM versions from 16 to 19 (at least). The checker is suggesting to add O_CLOEXEC to 'mode', and not in 'flags', where we already have it. Add a suppression for clang-tidy and a comment, and avoid repeating those three times by adding a new helper, output_file_open(). Signed-off-by: Stefano Brivio Reviewed-by: David Gibson --- conf.c | 5 ++++- log.c | 3 +-- pcap.c | 7 ++----- util.c | 27 +++++++++++---------------- util.h | 2 +- 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/conf.c b/conf.c index 4db7c64..d6faa5e 100644 --- a/conf.c +++ b/conf.c @@ -1194,7 +1194,10 @@ static void conf_open_files(struct ctx *c) if (c->mode != MODE_PASTA && c->fd_tap == -1) c->fd_tap_listen = tap_sock_unix_open(c->sock_path); - c->pidfile_fd = pidfile_open(c->pidfile); + if (*c->pidfile) { + if ((c->pidfile_fd = output_file_open(c->pidfile, 0)) < 0) + die_perror("Couldn't open PID file %s", c->pidfile); + } } /** diff --git a/log.c b/log.c index 6932885..0adddff 100644 --- a/log.c +++ b/log.c @@ -416,8 +416,7 @@ void logfile_init(const char *name, const char *path, size_t size) if (readlink("/proc/self/exe", exe, PATH_MAX - 1) < 0) die_perror("Failed to read own /proc/self/exe link"); - log_file = open(path, O_CREAT | O_TRUNC | O_APPEND | O_RDWR | O_CLOEXEC, - S_IRUSR | S_IWUSR); + log_file = output_file_open(path, O_APPEND); if (log_file == -1) die_perror("Couldn't open log file %s", path); diff --git a/pcap.c b/pcap.c index 6ee6cdf..12737d8 100644 --- a/pcap.c +++ b/pcap.c @@ -158,18 +158,15 @@ void pcap_iov(const struct iovec *iov, size_t iovcnt, size_t offset) */ void pcap_init(struct ctx *c) { - int flags = O_WRONLY | O_CREAT | O_TRUNC; - if (pcap_fd != -1) return; if (!*c->pcap) return; - flags |= c->foreground ? O_CLOEXEC : 0; - pcap_fd = open(c->pcap, flags, S_IRUSR | S_IWUSR); + pcap_fd = output_file_open(c->pcap, 0); if (pcap_fd == -1) { - perror("open"); + err_perror("Couldn't open pcap file %s", c->pcap); return; } diff --git a/util.c b/util.c index 21ce0a8..944b110 100644 --- a/util.c +++ b/util.c @@ -407,25 +407,20 @@ void pidfile_write(int fd, pid_t pid) } /** - * pidfile_open() - Open PID file if needed - * @path: Path for PID file, empty string if no PID file is requested + * output_file_open() - Open file for output, if needed + * @path: Path for output file + * @flags: Additional flags for open() * - * Return: descriptor for PID file, -1 if path is NULL, won't return on failure + * Return: file descriptor on success, -1 on failure with errno set by open() */ -int pidfile_open(const char *path) +int output_file_open(const char *path, int flags) { - int fd; - - if (!*path) - return -1; - - if ((fd = open(path, O_CREAT | O_TRUNC | O_WRONLY | O_CLOEXEC, - S_IRUSR | S_IWUSR)) < 0) { - perror("PID file open"); - exit(EXIT_FAILURE); - } - - return fd; + /* We use O_CLOEXEC here, but clang-tidy as of LLVM 16 to 19 looks for + * it in the 'mode' argument if we have one + */ + return open(path, O_CREAT | O_TRUNC | O_WRONLY | O_CLOEXEC | flags, + /* NOLINTNEXTLINE(android-cloexec-open) */ + S_IRUSR | S_IWUSR); } /** diff --git a/util.h b/util.h index 4f8b768..3fc64cf 100644 --- a/util.h +++ b/util.h @@ -193,7 +193,7 @@ char *line_read(char *buf, size_t len, int fd); void ns_enter(const struct ctx *c); bool ns_is_init(void); int open_in_ns(const struct ctx *c, const char *path, int flags); -int pidfile_open(const char *path); +int output_file_open(const char *path, int flags); void pidfile_write(int fd, pid_t pid); int __daemon(int pidfile_fd, int devnull_fd); int fls(unsigned long x); -- 2.43.0