diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 9d99321a2..eeefa1e3c 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -1,28 +1,30 @@ // The utility library for universal variables. Used both by the client library and by the daemon. -#include "config.h" +#include "config.h" // IWYU pragma: keep #include // IWYU pragma: keep #include #include #include #include -#include +#include // IWYU pragma: keep #include #include #include +#ifdef __CYGWIN__ #include +#endif +#ifdef HAVE_SYS_SELECT_H +#include // IWYU pragma: keep +#endif #include #include -#include +#include // IWYU pragma: keep #include // We need the sys/file.h for the flock() declaration on Linux but not OS X. #include // IWYU pragma: keep // We need the ioctl.h header so we can check if SIOCGIFHWADDR is defined by it so we know if we're // on a Linux system. #include // IWYU pragma: keep -#ifdef HAVE_SYS_SELECT_H -#include -#endif #include #include #include @@ -35,7 +37,7 @@ #include "fallback.h" // IWYU pragma: keep #include "path.h" #include "utf8.h" -#include "util.h" +#include "util.h" // IWYU pragma: keep #include "wutil.h" #if __APPLE__ @@ -950,8 +952,8 @@ wcstring get_machine_identifier() { return result; } -#ifdef HAVE_SHM_OPEN class universal_notifier_shmem_poller_t : public universal_notifier_t { +#ifdef __CYGWIN__ // This is what our shared memory looks like. Everything here is stored in network byte order // (big-endian). struct universal_notifier_shmem_t { @@ -1089,17 +1091,22 @@ class universal_notifier_shmem_poller_t : public universal_notifier_t { } return usec_per_sec / 3; // 3 times a second } -}; +#else // this class isn't valid on this system + public: + universal_notifier_shmem_poller_t() { + DIE("universal_notifier_shmem_poller_t cannot be used on this system"); + } #endif +}; /// A notifyd-based notifier. Very straightforward. class universal_notifier_notifyd_t : public universal_notifier_t { +#if FISH_NOTIFYD_AVAILABLE int notify_fd; int token; std::string name; void setup_notifyd() { -#if FISH_NOTIFYD_AVAILABLE // Per notify(3), the user.uid.%d style is only accessible to processes with that uid. char local_name[256]; snprintf(local_name, sizeof local_name, "user.uid.%d.%ls.uvars", getuid(), @@ -1128,7 +1135,6 @@ class universal_notifier_notifyd_t : public universal_notifier_t { // marked as CLO_EXEC, that's probably a good thing. set_cloexec(this->notify_fd + 1); } -#endif } public: @@ -1138,9 +1144,7 @@ class universal_notifier_notifyd_t : public universal_notifier_t { ~universal_notifier_notifyd_t() { if (token != -1 /* NOTIFY_TOKEN_INVALID */) { -#if FISH_NOTIFYD_AVAILABLE notify_cancel(token); -#endif } } @@ -1161,16 +1165,18 @@ class universal_notifier_notifyd_t : public universal_notifier_t { } void post_notification() { -#if FISH_NOTIFYD_AVAILABLE uint32_t status = notify_post(name.c_str()); if (status != NOTIFY_STATUS_OK) { - fprintf(stderr, - "Warning: notify_post() failed with status %u. Universal variable " - "notifications may not be sent.", - status); + debug(1, "notify_post() failed with status %u. Uvar notifications may not be sent.", + status); } -#endif } +#else // this class isn't valid on this system + public: + universal_notifier_notifyd_t() { + DIE("universal_notifier_notifyd_t cannot be used on this system"); + } +#endif }; #define NAMED_PIPE_FLASH_DURATION_USEC (1000000 / 10) @@ -1186,6 +1192,7 @@ class universal_notifier_notifyd_t : public universal_notifier_t { // when there is data remaining in the pipe, if the pipe is kept readable too long, clients will // attempt to read data out of it (to render it no longer readable). class universal_notifier_named_pipe_t : public universal_notifier_t { +#if !defined(__APPLE__) && !defined(__CYGWIN__) int pipe_fd; long long readback_time_usec; size_t readback_amount; @@ -1206,7 +1213,7 @@ class universal_notifier_named_pipe_t : public universal_notifier_t { } public: - explicit universal_notifier_named_pipe_t(const wchar_t *test_path) + universal_notifier_named_pipe_t(const wchar_t *test_path) : pipe_fd(-1), readback_time_usec(0), readback_amount(0), @@ -1332,54 +1339,15 @@ class universal_notifier_named_pipe_t : public universal_notifier_t { } return true; } +#else // this class isn't valid on this system + public: + universal_notifier_named_pipe_t(const wchar_t *test_path) { + DIE("universal_notifier_named_pipe_t cannot be used on this system"); + } +#endif }; -class universal_notifier_null_t : public universal_notifier_t {}; // does nothing - -static universal_notifier_t::notifier_strategy_t fetch_default_strategy_from_environment() { - universal_notifier_t::notifier_strategy_t result = universal_notifier_t::strategy_default; - - const struct { - const char *name; - universal_notifier_t::notifier_strategy_t strat; - } options[] = {{"default", universal_notifier_t::strategy_default}, -#ifdef HAVE_SHM_OPEN - {"shmem", universal_notifier_t::strategy_shmem_polling}, -#endif - {"pipe", universal_notifier_t::strategy_named_pipe}, - {"notifyd", universal_notifier_t::strategy_notifyd}}; - const size_t opt_count = sizeof options / sizeof *options; - - const char *var = getenv(UNIVERSAL_NOTIFIER_ENV_NAME); - if (var == NULL || var[0] == '\0') { - return result; - } - - size_t i; - for (i = 0; i < opt_count; i++) { - if (!strcmp(var, options[i].name)) { - result = options[i].strat; - break; - } - } - if (i >= opt_count) { - fprintf(stderr, "Warning: unrecognized value for %s: '%s'\n", UNIVERSAL_NOTIFIER_ENV_NAME, - var); - fprintf(stderr, "Warning: valid values are "); - for (size_t j = 0; j < opt_count; j++) { - fprintf(stderr, "%s%s", j > 0 ? ", " : "", options[j].name); - } - fputc('\n', stderr); - } - return result; -} - universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_strategy() { - static universal_notifier_t::notifier_strategy_t s_explicit_strategy = - fetch_default_strategy_from_environment(); - if (s_explicit_strategy != strategy_default) { - return s_explicit_strategy; - } #if FISH_NOTIFYD_AVAILABLE return strategy_notifyd; #elif defined(__CYGWIN__) @@ -1390,32 +1358,25 @@ universal_notifier_t::notifier_strategy_t universal_notifier_t::resolve_default_ } universal_notifier_t &universal_notifier_t::default_notifier() { - static universal_notifier_t *result = new_notifier_for_strategy(strategy_default); + static universal_notifier_t *result = + new_notifier_for_strategy(universal_notifier_t::resolve_default_strategy()); return *result; } universal_notifier_t *universal_notifier_t::new_notifier_for_strategy( universal_notifier_t::notifier_strategy_t strat, const wchar_t *test_path) { - if (strat == strategy_default) { - strat = resolve_default_strategy(); //!OCLINT(parameter reassignment) - } switch (strat) { -#ifdef HAVE_SHM_OPEN - case strategy_shmem_polling: { - return new universal_notifier_shmem_poller_t(); - } -#endif case strategy_notifyd: { return new universal_notifier_notifyd_t(); } + case strategy_shmem_polling: { + return new universal_notifier_shmem_poller_t(); + } case strategy_named_pipe: { return new universal_notifier_named_pipe_t(test_path); } - case strategy_null: { - return new universal_notifier_null_t(); - } default: { - fprintf(stderr, "Unsupported strategy %d\n", strat); + debug(0, "Unsupported universal notifier strategy %d\n", strat); return NULL; } } @@ -1439,6 +1400,7 @@ bool universal_notifier_t::notification_fd_became_readable(int fd) { return false; } +#if !defined(__APPLE__) && !defined(__CYGWIN__) void universal_notifier_named_pipe_t::make_pipe(const wchar_t *test_path) { wcstring vars_path = test_path ? wcstring(test_path) : default_named_pipe_path(); vars_path.append(L".notifier"); @@ -1464,3 +1426,4 @@ void universal_notifier_named_pipe_t::make_pipe(const wchar_t *test_path) { pipe_fd = fd; } +#endif diff --git a/src/env_universal_common.h b/src/env_universal_common.h index d574b6132..c25c7e2e6 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -1,6 +1,6 @@ #ifndef FISH_ENV_UNIVERSAL_COMMON_H #define FISH_ENV_UNIVERSAL_COMMON_H -#include "config.h" +#include "config.h" // IWYU pragma: keep #include #include @@ -113,24 +113,14 @@ class env_universal_t { class universal_notifier_t { public: enum notifier_strategy_t { - // Default meta-strategy to use the 'best' notifier for the system. - strategy_default, - -#ifdef HAVE_SHM_OPEN // Use a value in shared memory. Simple, but requires polling and therefore semi-frequent // wakeups. strategy_shmem_polling, -#endif - + // Strategy that uses notify(3). Simple and efficient, but OS X/macOS only. + strategy_notifyd, // Strategy that uses a named pipe. Somewhat complex, but portable and doesn't require // polling most of the time. strategy_named_pipe, - - // Strategy that uses notify(3). Simple and efficient, but OS X only. - strategy_notifyd, - - // Null notifier, does nothing. - strategy_null }; protected: @@ -140,9 +130,9 @@ class universal_notifier_t { // No copying. universal_notifier_t &operator=(const universal_notifier_t &); universal_notifier_t(const universal_notifier_t &x); - static notifier_strategy_t resolve_default_strategy(); public: + static notifier_strategy_t resolve_default_strategy(); virtual ~universal_notifier_t(); // Factory constructor. Free with delete. diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index ac9616eb7..f19a6783f 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -25,11 +25,8 @@ #include #include #include -#include -#include #include #include -#include #include #include @@ -1648,7 +1645,7 @@ struct pager_layout_testcase_t { wcstring text = sd.line(0).to_string(); if (text != expected) { - fprintf(stderr, "width %lu got <%ls>, expected <%ls>\n", this->width, text.c_str(), + fprintf(stderr, "width %zu got <%ls>, expected <%ls>\n", this->width, text.c_str(), expected.c_str()); } do_test(text == expected); @@ -2583,37 +2580,24 @@ bool poll_notifier(universal_notifier_t *note) { return result; } -static void trigger_or_wait_for_notification(universal_notifier_t *notifier, - universal_notifier_t::notifier_strategy_t strategy) { - // This argument should be removed if it isn't actually needed by these unit tests. I'm going to - // silence the warning. See https://github.com/fish-shell/fish-shell/issues/3439. - UNUSED(notifier); - +static void trigger_or_wait_for_notification(universal_notifier_t::notifier_strategy_t strategy) { switch (strategy) { - case universal_notifier_t::strategy_default: { - DIE("strategy_default should be passed"); - break; - } -#ifdef HAVE_SHM_OPEN case universal_notifier_t::strategy_shmem_polling: { break; // nothing required } -#endif case universal_notifier_t::strategy_notifyd: { // notifyd requires a round trip to the notifyd server, which means we have to wait a - // little bit to receive it. In practice, this seems to be enough. - usleep(1000000 / 25); + // little bit to receive it. In practice 40 ms seems to be enough. + usleep(40000); break; } - case universal_notifier_t::strategy_named_pipe: - case universal_notifier_t::strategy_null: { - break; + case universal_notifier_t::strategy_named_pipe: { + break; // nothing required } } } static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy_t strategy) { - assert(strategy != universal_notifier_t::strategy_default); say(L"Testing universal notifiers with strategy %d", (int)strategy); universal_notifier_t *notifiers[16]; size_t notifier_count = sizeof notifiers / sizeof *notifiers; @@ -2636,7 +2620,7 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy notifiers[post_idx]->post_notification(); // Do special stuff to "trigger" a notification for testing. - trigger_or_wait_for_notification(notifiers[post_idx], strategy); + trigger_or_wait_for_notification(strategy); for (size_t i = 0; i < notifier_count; i++) { // We aren't concerned with the one who posted. Poll from it (to drain it), and then @@ -2680,15 +2664,12 @@ static void test_notifiers_with_strategy(universal_notifier_t::notifier_strategy } static void test_universal_notifiers() { - if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) + if (system("mkdir -p /tmp/fish_uvars_test/ && touch /tmp/fish_uvars_test/varsfile.txt")) { err(L"mkdir failed"); -#ifdef HAVE_SHM_OPEN - test_notifiers_with_strategy(universal_notifier_t::strategy_shmem_polling); -#endif - test_notifiers_with_strategy(universal_notifier_t::strategy_named_pipe); -#if __APPLE__ - test_notifiers_with_strategy(universal_notifier_t::strategy_notifyd); -#endif + } + + auto strategy = universal_notifier_t::resolve_default_strategy(); + test_notifiers_with_strategy(strategy); if (system("rm -Rf /tmp/fish_uvars_test/")) err(L"rm failed"); }