From ee46dba838da69e066232af79fe373a8533a7bc2 Mon Sep 17 00:00:00 2001 From: Steffen Winter Date: Fri, 9 Feb 2024 12:42:54 +0100 Subject: [PATCH 1/2] bsd: Wrap kvm_t* in a uniq_ptr and share code between BSD's --- src/btop_shared.hpp | 20 +++++++++++++++++++- src/freebsd/btop_collect.cpp | 22 +++++----------------- src/openbsd/btop_collect.cpp | 18 +++--------------- 3 files changed, 27 insertions(+), 33 deletions(-) diff --git a/src/btop_shared.hpp b/src/btop_shared.hpp index 69566c6..e60da98 100644 --- a/src/btop_shared.hpp +++ b/src/btop_shared.hpp @@ -25,10 +25,19 @@ tab-size = 4 #include #include #include -#include #include #include +// From `man 3 getifaddrs`: must be included before +// clang-format off +#include +#include +// clang-format on + +#if defined(__FreeBSD__) || defined(__OpenBSD__) +# include +#endif + using std::array; using std::atomic; using std::deque; @@ -83,6 +92,15 @@ namespace Shared { void init(); extern long coreCount, page_size, clk_tck; + +#if defined(__FreeBSD__) || defined(__OpenBSD__) + struct KvmDeleter { + void operator()(kvm_t* handle) { + kvm_close(handle); + } + }; + using KvmPtr = std::unique_ptr; +#endif } diff --git a/src/freebsd/btop_collect.cpp b/src/freebsd/btop_collect.cpp index 2db49a2..094d78a 100644 --- a/src/freebsd/btop_collect.cpp +++ b/src/freebsd/btop_collect.cpp @@ -189,18 +189,6 @@ namespace Shared { Logger::debug("Init -> Mem::get_zpools()"); Mem::get_zpools(); } - - //* RAII wrapper for kvm_openfiles - class kvm_openfiles_wrapper { - kvm_t* kd = nullptr; - public: - kvm_openfiles_wrapper(const char* execf, const char* coref, const char* swapf, int flags, char* err) { - this->kd = kvm_openfiles(execf, coref, swapf, flags, err); - } - ~kvm_openfiles_wrapper() { kvm_close(kd); } - auto operator()() -> kvm_t* { return kd; } - }; - } // namespace Shared namespace Cpu { @@ -668,9 +656,9 @@ namespace Mem { if (show_swap) { char buf[_POSIX2_LINE_MAX]; - Shared::kvm_openfiles_wrapper kd(nullptr, _PATH_DEVNULL, nullptr, O_RDONLY, buf); + Shared::KvmPtr kd {kvm_openfiles(nullptr, _PATH_DEVNULL, nullptr, O_RDONLY, buf)}; struct kvm_swap swap[16]; - int nswap = kvm_getswapinfo(kd(), swap, 16, 0); + int nswap = kvm_getswapinfo(kd.get(), swap, 16, 0); int totalSwap = 0, usedSwap = 0; for (int i = 0; i < nswap; i++) { totalSwap += swap[i].ksw_total; @@ -1164,8 +1152,8 @@ namespace Proc { int count = 0; char buf[_POSIX2_LINE_MAX]; - Shared::kvm_openfiles_wrapper kd(nullptr, _PATH_DEVNULL, nullptr, O_RDONLY, buf); - const struct kinfo_proc* kprocs = kvm_getprocs(kd(), KERN_PROC_PROC, 0, &count); + Shared::KvmPtr kd {kvm_openfiles(nullptr, _PATH_DEVNULL, nullptr, O_RDONLY, buf)}; + const struct kinfo_proc* kprocs = kvm_getprocs(kd.get(), KERN_PROC_PROC, 0, &count); for (int i = 0; i < count; i++) { const struct kinfo_proc* kproc = &kprocs[i]; @@ -1192,7 +1180,7 @@ namespace Proc { continue; } new_proc.name = kproc->ki_comm; - char** argv = kvm_getargv(kd(), kproc, 0); + char** argv = kvm_getargv(kd.get(), kproc, 0); if (argv) { for (int i = 0; argv[i] and cmp_less(new_proc.cmd.size(), 1000); i++) { new_proc.cmd += argv[i] + " "s; diff --git a/src/openbsd/btop_collect.cpp b/src/openbsd/btop_collect.cpp index df35662..cc24388 100644 --- a/src/openbsd/btop_collect.cpp +++ b/src/openbsd/btop_collect.cpp @@ -184,18 +184,6 @@ namespace Shared { Mem::old_uptime = system_uptime(); Mem::collect(); } - - //* RAII wrapper for kvm_openfiles - class kvm_openfiles_wrapper { - kvm_t* kd = nullptr; - public: - kvm_openfiles_wrapper(const char* execf, const char* coref, const char* swapf, int flags, char* err) { - this->kd = kvm_openfiles(execf, coref, swapf, flags, err); - } - ~kvm_openfiles_wrapper() { kvm_close(kd); } - auto operator()() -> kvm_t* { return kd; } - }; - } // namespace Shared namespace Cpu { @@ -1102,8 +1090,8 @@ namespace Proc { int count = 0; char buf[_POSIX2_LINE_MAX]; - Shared::kvm_openfiles_wrapper kd(nullptr, nullptr, nullptr, KVM_NO_FILES, buf); - const struct kinfo_proc* kprocs = kvm_getprocs(kd(), KERN_PROC_ALL, 0, sizeof(struct kinfo_proc), &count); + Shared::KvmPtr kd {kvm_openfiles(nullptr, nullptr, nullptr, KVM_NO_FILES, buf)}; + const struct kinfo_proc* kprocs = kvm_getprocs(kd.get() , KERN_PROC_ALL, 0, sizeof(struct kinfo_proc), &count); for (int i = 0; i < count; i++) { const struct kinfo_proc* kproc = &kprocs[i]; @@ -1130,7 +1118,7 @@ namespace Proc { continue; } new_proc.name = kproc->p_comm; - char** argv = kvm_getargv(kd(), kproc, 0); + char** argv = kvm_getargv(kd.get(), kproc, 0); if (argv) { for (int i = 0; argv[i] and cmp_less(new_proc.cmd.size(), 1000); i++) { new_proc.cmd += argv[i] + " "s; From ab0bef204a248609ac05948e531108a8032b8e63 Mon Sep 17 00:00:00 2001 From: Steffen Winter Date: Fri, 9 Feb 2024 13:53:42 +0100 Subject: [PATCH 2/2] collect: Share ifaddrs wrapper and use uniq_ptr-like syntax --- src/btop_shared.hpp | 11 +++++++++++ src/freebsd/btop_collect.cpp | 19 ++++--------------- src/linux/btop_collect.cpp | 18 ++++-------------- src/openbsd/btop_collect.cpp | 19 ++++--------------- 4 files changed, 23 insertions(+), 44 deletions(-) diff --git a/src/btop_shared.hpp b/src/btop_shared.hpp index e60da98..8031321 100644 --- a/src/btop_shared.hpp +++ b/src/btop_shared.hpp @@ -307,6 +307,17 @@ namespace Net { bool connected{}; }; + class IfAddrsPtr { + struct ifaddrs* ifaddr; + int status; + public: + IfAddrsPtr() { status = getifaddrs(&ifaddr); } + ~IfAddrsPtr() { freeifaddrs(ifaddr); } + [[nodiscard]] constexpr auto operator()() -> struct ifaddrs* { return ifaddr; } + [[nodiscard]] constexpr auto get() -> struct ifaddrs* { return ifaddr; } + [[nodiscard]] constexpr auto get_status() const noexcept -> int { return status; }; + }; + extern std::unordered_map current_net; //* Collect net upload/download stats diff --git a/src/freebsd/btop_collect.cpp b/src/freebsd/btop_collect.cpp index 094d78a..d64a878 100644 --- a/src/freebsd/btop_collect.cpp +++ b/src/freebsd/btop_collect.cpp @@ -811,17 +811,6 @@ namespace Net { bool rescale = true; uint64_t timestamp = 0; - //* RAII wrapper for getifaddrs - class getifaddr_wrapper { - struct ifaddrs *ifaddr; - - public: - int status; - getifaddr_wrapper() { status = getifaddrs(&ifaddr); } - ~getifaddr_wrapper() { freeifaddrs(ifaddr); } - auto operator()() -> struct ifaddrs * { return ifaddr; } - }; - auto collect(bool no_update) -> net_info & { auto &net = current_net; auto &config_iface = Config::getS("net_iface"); @@ -831,10 +820,10 @@ namespace Net { if (not no_update and errors < 3) { //? Get interface list using getifaddrs() wrapper - getifaddr_wrapper if_wrap{}; - if (if_wrap.status != 0) { + IfAddrsPtr if_addrs {}; + if (if_addrs.get_status() != 0) { errors++; - Logger::error("Net::collect() -> getifaddrs() failed with id " + to_string(if_wrap.status)); + Logger::error("Net::collect() -> getifaddrs() failed with id " + to_string(if_addrs.get_status())); redraw = true; return empty_net; } @@ -846,7 +835,7 @@ namespace Net { string ipv4, ipv6; //? Iteration over all items in getifaddrs() list - for (auto *ifa = if_wrap(); ifa != nullptr; ifa = ifa->ifa_next) { + for (auto *ifa = if_addrs.get(); ifa != nullptr; ifa = ifa->ifa_next) { if (ifa->ifa_addr == nullptr) continue; family = ifa->ifa_addr->sa_family; const auto &iface = ifa->ifa_name; diff --git a/src/linux/btop_collect.cpp b/src/linux/btop_collect.cpp index eb8c2b7..7096ad9 100644 --- a/src/linux/btop_collect.cpp +++ b/src/linux/btop_collect.cpp @@ -2151,16 +2151,6 @@ namespace Net { bool rescale{true}; uint64_t timestamp{}; - //* RAII wrapper for getifaddrs - class getifaddr_wrapper { - struct ifaddrs* ifaddr; - public: - int status; - getifaddr_wrapper() { status = getifaddrs(&ifaddr); } - ~getifaddr_wrapper() { freeifaddrs(ifaddr); } - auto operator()() -> struct ifaddrs* { return ifaddr; } - }; - auto collect(bool no_update) -> net_info& { if (Runner::stopping) return empty_net; auto& net = current_net; @@ -2171,10 +2161,10 @@ namespace Net { if (not no_update and errors < 3) { //? Get interface list using getifaddrs() wrapper - getifaddr_wrapper if_wrap {}; - if (if_wrap.status != 0) { + IfAddrsPtr if_addrs {}; + if (if_addrs.get_status() != 0) { errors++; - Logger::error("Net::collect() -> getifaddrs() failed with id " + to_string(if_wrap.status)); + Logger::error("Net::collect() -> getifaddrs() failed with id " + to_string(if_addrs.get_status())); redraw = true; return empty_net; } @@ -2186,7 +2176,7 @@ namespace Net { string ipv4, ipv6; //? Iteration over all items in getifaddrs() list - for (auto* ifa = if_wrap(); ifa != nullptr; ifa = ifa->ifa_next) { + for (auto* ifa = if_addrs.get(); ifa != nullptr; ifa = ifa->ifa_next) { if (ifa->ifa_addr == nullptr) continue; family = ifa->ifa_addr->sa_family; const auto& iface = ifa->ifa_name; diff --git a/src/openbsd/btop_collect.cpp b/src/openbsd/btop_collect.cpp index cc24388..b0d36a5 100644 --- a/src/openbsd/btop_collect.cpp +++ b/src/openbsd/btop_collect.cpp @@ -768,17 +768,6 @@ namespace Net { bool rescale = true; uint64_t timestamp = 0; - //* RAII wrapper for getifaddrs - class getifaddr_wrapper { - struct ifaddrs *ifaddr; - - public: - int status; - getifaddr_wrapper() { status = getifaddrs(&ifaddr); } - ~getifaddr_wrapper() { freeifaddrs(ifaddr); } - auto operator()() -> struct ifaddrs * { return ifaddr; } - }; - auto collect(bool no_update) -> net_info & { auto &net = current_net; auto &config_iface = Config::getS("net_iface"); @@ -788,10 +777,10 @@ namespace Net { if (not no_update and errors < 3) { //? Get interface list using getifaddrs() wrapper - getifaddr_wrapper if_wrap{}; - if (if_wrap.status != 0) { + IfAddrsPtr if_addrs {}; + if (if_addrs.get_status() != 0) { errors++; - Logger::error("Net::collect() -> getifaddrs() failed with id " + to_string(if_wrap.status)); + Logger::error("Net::collect() -> getifaddrs() failed with id " + to_string(if_addrs.get_status())); redraw = true; return empty_net; } @@ -803,7 +792,7 @@ namespace Net { string ipv4, ipv6; //? Iteration over all items in getifaddrs() list - for (auto *ifa = if_wrap(); ifa != nullptr; ifa = ifa->ifa_next) { + for (auto *ifa = if_addrs.get(); ifa != nullptr; ifa = ifa->ifa_next) { if (ifa->ifa_addr == nullptr) continue; family = ifa->ifa_addr->sa_family; const auto &iface = ifa->ifa_name;