From fc4557c78494f5bfc94d34b13bf93b91f152e3f8 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 28 Jan 2020 10:43:37 -0800 Subject: [PATCH] Eliminate wopen() It was large and mostly unnecessary. Prefer wopen() followed by fdopen(). --- src/fish_tests.cpp | 2 +- src/proc.cpp | 16 +++++++++------- src/reader.cpp | 5 +++-- src/wutil.cpp | 36 ------------------------------------ src/wutil.h | 3 --- 5 files changed, 13 insertions(+), 49 deletions(-) diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index a1a6cbad6..2c3236983 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -3369,7 +3369,7 @@ static void test_universal_ok_to_save() { say(L"Testing universal Ok to save"); if (system("mkdir -p test/fish_uvars_test/")) err(L"mkdir failed"); const char *contents = "# VERSION: 99999.99\n"; - FILE *fp = wfopen(UVARS_TEST_PATH, "w"); + FILE *fp = fopen(wcs2string(UVARS_TEST_PATH).c_str(), "w"); assert(fp && "Failed to open UVARS_TEST_PATH for writing"); fwrite(contents, std::strlen(contents), 1, fp); fclose(fp); diff --git a/src/proc.cpp b/src/proc.cpp index 6550d43e2..d4e501add 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -7,6 +7,7 @@ #include "config.h" #include +#include #include #include #include @@ -620,15 +621,11 @@ bool job_reap(parser_t &parser, bool allow_interactive) { return printed; } -/// Maximum length of a /proc/[PID]/stat filename. -#define FN_SIZE 256 - /// Get the CPU time for the specified process. unsigned long proc_get_jiffies(process_t *p) { if (!have_proc_stat()) return 0; if (p->pid <= 0) return 0; - wchar_t fn[FN_SIZE]; char state; int pid, ppid, pgrp, session, tty_nr, tpgid, exit_signal, processor; long int cutime, cstime, priority, nice, placeholder, itrealvalue, rss; @@ -637,11 +634,16 @@ unsigned long proc_get_jiffies(process_t *p) { wchan, nswap, cnswap; char comm[1024]; - std::swprintf(fn, FN_SIZE, L"/proc/%d/stat", p->pid); - FILE *f = wfopen(fn, "r"); - if (!f) return 0; + /// Maximum length of a /proc/[PID]/stat filename. + constexpr size_t FN_SIZE = 256; + char fn[FN_SIZE]; + std::snprintf(fn, FN_SIZE, "/proc/%d/stat", p->pid); + // Don't use autoclose_fd here, we will fdopen() and then fclose() instead. + int fd = open_cloexec(fn, O_RDONLY); + if (fd < 0) return 0; // TODO: replace the use of fscanf() as it is brittle and should never be used. + FILE *f = fdopen(fd, "r"); int count = fscanf(f, "%9d %1023s %c %9d %9d %9d %9d %9d %9lu " "%9lu %9lu %9lu %9lu %9lu %9lu %9ld %9ld %9ld " diff --git a/src/reader.cpp b/src/reader.cpp index f4d3fe1a4..7e563b096 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -2174,8 +2174,9 @@ void reader_import_history_if_necessary() { const auto var = data->vars().get(L"HISTFILE"); wcstring path = (var ? var->as_string() : L"~/.bash_history"); expand_tilde(path, data->vars()); - FILE *f = wfopen(path, "r"); - if (f) { + int fd = wopen_cloexec(path, O_RDONLY); + if (fd >= 0) { + FILE *f = fdopen(fd, "r"); data->history->populate_from_bash(f); fclose(f); } diff --git a/src/wutil.cpp b/src/wutil.cpp index a136ce0ff..ce47bff8f 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -141,42 +141,6 @@ wcstring wgetcwd() { return wcstring(); } -FILE *wfopen(const wcstring &path, const char *mode) { - int permissions = 0, options = 0; - size_t idx = 0; - switch (mode[idx++]) { - case 'r': { - permissions = O_RDONLY; - break; - } - case 'w': { - permissions = O_WRONLY; - options = O_CREAT | O_TRUNC; - break; - } - case 'a': { - permissions = O_WRONLY; - options = O_CREAT | O_APPEND; - break; - } - default: { - errno = EINVAL; - return nullptr; - } - } - // Skip binary. - if (mode[idx] == 'b') idx++; - - // Consider append option. - if (mode[idx] == '+') permissions = O_RDWR; - - int fd = wopen_cloexec(path, permissions | options, 0666); - if (fd < 0) return nullptr; - FILE *result = fdopen(fd, mode); - if (result == nullptr) close(fd); - return result; -} - int set_cloexec(int fd, bool should_set) { // Note we don't want to overwrite existing flags like O_NONBLOCK which may be set. So fetch the // existing flags and modify them. diff --git a/src/wutil.h b/src/wutil.h index 7e9c7ca2c..c142e6853 100644 --- a/src/wutil.h +++ b/src/wutil.h @@ -19,9 +19,6 @@ #include "common.h" #include "maybe.h" -/// Wide character version of fopen(). This sets CLO_EXEC. -FILE *wfopen(const wcstring &path, const char *mode); - /// Sets CLO_EXEC on a given fd according to the value of \p should_set. int set_cloexec(int fd, bool should_set = true);