Cleanup use of pointers in util code

This commit is contained in:
patrick96 2022-03-06 17:44:48 +01:00 committed by Patrick Ziegler
parent 8ddf9d2cdf
commit 50eac859fd
26 changed files with 117 additions and 197 deletions

View File

@ -268,9 +268,4 @@ struct event_timer {
};
};
enum class output_policy {
REDIRECTED,
IGNORED,
};
POLYBAR_NS_END

View File

@ -23,7 +23,7 @@ namespace modules {
explicit backlight_module(const bar_settings&, string);
void idle();
bool on_event(inotify_event* event);
bool on_event(const inotify_event& event);
bool build(builder* builder, const string& tag) const;
static constexpr auto TYPE = "internal/backlight";

View File

@ -51,7 +51,7 @@ namespace modules {
void start() override;
void teardown();
void idle();
bool on_event(inotify_event* event);
bool on_event(const inotify_event& event);
string get_format() const;
bool build(builder* builder, const string& tag) const;

View File

@ -35,7 +35,7 @@ namespace modules {
string m_api_url;
string m_user;
string m_accesstoken{};
unique_ptr<http_downloader> m_http{};
http_downloader m_http{};
bool m_empty_notifications{false};
std::atomic<bool> m_offline{false};
};

View File

@ -22,7 +22,7 @@ namespace modules {
try {
// Warm up module output before entering the loop
std::unique_lock<std::mutex> guard(this->m_updatelock);
CAST_MOD(Impl)->on_event(nullptr);
CAST_MOD(Impl)->on_event({});
CAST_MOD(Impl)->broadcast();
guard.unlock();
@ -47,12 +47,12 @@ namespace modules {
}
void poll_events() {
vector<unique_ptr<inotify_watch>> watches;
vector<inotify_watch> watches;
try {
for (auto&& w : m_watchlist) {
watches.emplace_back(inotify_util::make_watch(w.first));
watches.back()->attach(w.second);
watches.emplace_back(w.first);
watches.back().attach(w.second);
}
} catch (const system_error& e) {
watches.clear();
@ -63,16 +63,16 @@ namespace modules {
while (this->running()) {
for (auto&& w : watches) {
this->m_log.trace_x("%s: Poll inotify watch %s", this->name(), w->path());
this->m_log.trace_x("%s: Poll inotify watch %s", this->name(), w.path());
if (w->poll(1000 / watches.size())) {
auto event = w->get_event();
if (w.poll(1000 / watches.size())) {
auto event = w.get_event();
for (auto&& w : watches) {
w->remove(true);
w.remove(true);
}
if (CAST_MOD(Impl)->on_event(event.get())) {
if (CAST_MOD(Impl)->on_event(event)) {
CAST_MOD(Impl)->broadcast();
}
CAST_MOD(Impl)->idle();

View File

@ -10,32 +10,37 @@ POLYBAR_NS
DEFINE_ERROR(command_error);
enum class output_policy {
REDIRECTED,
IGNORED,
};
/**
* Wrapper used to execute command in a subprocess.
* In-/output streams are opened to enable ipc.
* If the command is created using command_util::make_command<output_policy::REDIRECTED>, the child streams are
* If the command is created using command<output_policy::REDIRECTED>, the child streams are
* redirected and you can read the program output or write into the program input.
*
* If the command is created using command_util::make_command<output_policy::IGNORED>, the output is not redirected and
* If the command is created using command<output_policy::IGNORED>, the output is not redirected and
* you can't communicate with the child program.
*
* Example usage:
*
* @code cpp
* auto cmd = command_util::make_command<output_policy::REDIRECTED>("cat /etc/rc.local");
* command<output_policy::REDIRECTED>auto(m_log, "cat /etc/rc.local");
* cmd->exec();
* cmd->tail([](string s) { std::cout << s << std::endl; });
* @endcode
*
* @code cpp
* auto cmd = command_util::make_command<output_policy::REDIRECTED>("for i in 1 2 3; do echo $i; done");
* command<output_policy::REDIRECTED>auto(m_log, "for i in 1 2 3; do echo $i; done");
* cmd->exec();
* cout << cmd->readline(); // 1
* cout << cmd->readline() << cmd->readline(); // 23
* @endcode
*
* @code cpp
* auto cmd = command_util::make_command<output_policy::IGNORED>("ping kernel.org");
* command<output_policy::IGNORED>auto(m_log, "ping kernel.org");
* int status = cmd->exec();
* @endcode
*/
@ -98,11 +103,4 @@ class command<output_policy::REDIRECTED> : private command<output_policy::IGNORE
unique_ptr<fd_stream<std::istream>> m_stdout_reader{nullptr};
};
namespace command_util {
template <output_policy OutputType, typename... Args>
unique_ptr<command<OutputType>> make_command(Args&&... args) {
return std::make_unique<command<OutputType>>(logger::make(), forward<Args>(args)...);
}
} // namespace command_util
POLYBAR_NS_END

View File

@ -14,6 +14,9 @@ namespace this_thread = std::this_thread;
using std::mutex;
using std::thread;
/**
* Types wrapped in this type can be used as locks (e.g. for lock_guard).
*/
template <typename T>
class mutex_wrapper : public T {
public:

View File

@ -7,15 +7,6 @@
POLYBAR_NS
namespace factory_util {
namespace detail {
struct null_deleter {
template <typename T>
void operator()(T*) const {}
};
} // namespace detail
extern detail::null_deleter null_deleter;
template <class T, class... Deps>
shared_ptr<T> singleton(Deps&&... deps) {
static shared_ptr<T> instance{make_shared<T>(forward<Deps>(deps)...)};

View File

@ -1,12 +0,0 @@
#pragma once
#include <functional>
#include "common.hpp"
POLYBAR_NS
template <typename... Args>
using callback = function<void(Args...)>;
POLYBAR_NS_END

View File

@ -1,10 +1,11 @@
#pragma once
#include "common.hpp"
#include "utils/mixins.hpp"
POLYBAR_NS
class http_downloader {
class http_downloader : public non_copyable_mixin, public non_movable_mixin {
public:
http_downloader(int connection_timeout = 5);
~http_downloader();
@ -19,11 +20,4 @@ class http_downloader {
void* m_curl;
};
namespace http_util {
template <typename... Args>
decltype(auto) make_downloader(Args&&... args) {
return std::make_unique<http_downloader>(forward<Args>(args)...);
}
} // namespace http_util
POLYBAR_NS_END

View File

@ -10,6 +10,7 @@
POLYBAR_NS
struct inotify_event {
bool is_valid = false;
string filename;
bool is_dir;
int wd = 0;
@ -25,8 +26,7 @@ class inotify_watch {
void attach(int mask = IN_MODIFY);
void remove(bool force = false);
bool poll(int wait_ms = 1000) const;
unique_ptr<inotify_event> get_event() const;
unique_ptr<inotify_event> await_match() const;
inotify_event get_event() const;
const string path() const;
int get_file_descriptor() const;
@ -37,11 +37,4 @@ class inotify_watch {
int m_mask{0};
};
namespace inotify_util {
template <typename... Args>
decltype(auto) make_watch(Args&&... args) {
return std::make_unique<inotify_watch>(forward<Args>(args)...);
}
} // namespace inotify_util
POLYBAR_NS_END

View File

@ -26,7 +26,7 @@ namespace scope_util {
}
protected:
function<void()> m_callback;
function<void(void)> m_callback;
};
} // namespace scope_util

View File

@ -6,28 +6,6 @@
POLYBAR_NS
namespace {
/**
* Overload that allows sub-string removal at the end of given string
*/
inline string& operator-(string& a, const string& b) {
if (a.size() >= b.size() && a.substr(a.size() - b.size()) == b) {
return a.erase(a.size() - b.size());
} else {
return a;
}
}
/**
* Overload that allows sub-string removal at the end of given string
*/
inline void operator-=(string& a, const string& b) {
if (a.size() >= b.size() && a.substr(a.size() - b.size()) == b) {
a.erase(a.size() - b.size());
}
}
} // namespace
class sstream {
public:
sstream() : m_stream() {}

View File

@ -114,7 +114,6 @@ set(POLY_SOURCES
${src_dir}/utils/command.cpp
${src_dir}/utils/concurrency.cpp
${src_dir}/utils/env.cpp
${src_dir}/utils/factory.cpp
${src_dir}/utils/file.cpp
${src_dir}/utils/inotify.cpp
${src_dir}/utils/io.cpp

View File

@ -232,8 +232,8 @@ namespace net {
bool network::ping() const {
try {
auto exec = "ping -c 2 -W 2 -I " + m_interface + " " + string(CONNECTION_TEST_IP);
auto ping = command_util::make_command<output_policy::IGNORED>(exec);
return ping && ping->exec(true) == EXIT_SUCCESS;
command<output_policy::IGNORED> ping(m_log, exec);
return ping.exec(true) == EXIT_SUCCESS;
} catch (const std::exception& err) {
return false;
}
@ -279,8 +279,8 @@ namespace net {
* Get total net speed rate
*/
string network::netspeed(int minwidth, const string& unit) const {
float bytes_diff = m_status.current.received - m_status.previous.received
+ m_status.current.transmitted - m_status.previous.transmitted;
float bytes_diff = m_status.current.received - m_status.previous.received + m_status.current.transmitted -
m_status.previous.transmitted;
return format_speedrate(bytes_diff, minwidth, unit);
}

View File

@ -29,8 +29,8 @@ bool script_runner::check_condition() const {
return true;
}
auto exec_if_cmd = command_util::make_command<output_policy::IGNORED>(m_exec_if);
return exec_if_cmd->exec(true) == 0;
command<output_policy::IGNORED> exec_if_cmd(m_log, m_exec_if);
return exec_if_cmd.exec(true) == 0;
}
/**
@ -93,38 +93,38 @@ bool script_runner::set_output(string&& new_output) {
script_runner::interval script_runner::run() {
auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
m_log.info("script_runner: Invoking shell command: \"%s\"", exec);
auto cmd = command_util::make_command<output_policy::REDIRECTED>(exec);
command<output_policy::REDIRECTED> cmd(m_log, exec);
try {
cmd->exec(false, m_env);
cmd.exec(false, m_env);
} catch (const exception& err) {
m_log.err("script_runner: %s", err.what());
throw modules::module_error("Failed to execute command, stopping module...");
}
int fd = cmd->get_stdout(PIPE_READ);
int fd = cmd.get_stdout(PIPE_READ);
assert(fd != -1);
bool changed = false;
bool got_output = false;
while (!m_stopping && cmd->is_running() && !io_util::poll(fd, POLLHUP, 0)) {
while (!m_stopping && cmd.is_running() && !io_util::poll(fd, POLLHUP, 0)) {
/**
* For non-tailed scripts, we only use the first line. However, to ensure interruptability when the module shuts
* down, we still need to continue polling.
*/
if (io_util::poll_read(fd, 25) && !got_output) {
changed = set_output(cmd->readline());
changed = set_output(cmd.readline());
got_output = true;
}
}
if (m_stopping) {
cmd->terminate();
cmd.terminate();
return 0s;
}
m_exit_status = cmd->wait();
m_exit_status = cmd.wait();
if (!changed && m_exit_status != 0) {
clear_output();
@ -140,32 +140,32 @@ script_runner::interval script_runner::run() {
script_runner::interval script_runner::run_tail() {
auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
m_log.info("script_runner: Invoking shell command: \"%s\"", exec);
auto cmd = command_util::make_command<output_policy::REDIRECTED>(exec);
command<output_policy::REDIRECTED> cmd(m_log, exec);
try {
cmd->exec(false, m_env);
cmd.exec(false, m_env);
} catch (const exception& err) {
throw modules::module_error("Failed to execute command: " + string(err.what()));
}
scope_util::on_exit pid_guard([this]() { m_pid = -1; });
m_pid = cmd->get_pid();
m_pid = cmd.get_pid();
int fd = cmd->get_stdout(PIPE_READ);
int fd = cmd.get_stdout(PIPE_READ);
assert(fd != -1);
while (!m_stopping && cmd->is_running() && !io_util::poll(fd, POLLHUP, 0)) {
while (!m_stopping && cmd.is_running() && !io_util::poll(fd, POLLHUP, 0)) {
if (io_util::poll_read(fd, 25)) {
set_output(cmd->readline());
set_output(cmd.readline());
}
}
if (m_stopping) {
cmd->terminate();
cmd.terminate();
return 0s;
}
auto exit_status = cmd->wait();
auto exit_status = cmd.wait();
if (exit_status == 0) {
return m_interval;

View File

@ -71,9 +71,9 @@ namespace modules {
sleep(75ms);
}
bool backlight_module::on_event(inotify_event* event) {
if (event != nullptr) {
m_log.trace("%s: %s", name(), event->filename);
bool backlight_module::on_event(const inotify_event& event) {
if (event.is_valid) {
m_log.trace("%s: %s", name(), event.filename);
}
m_max_brightness = m_max.read();

View File

@ -1,14 +1,14 @@
#include "modules/battery.hpp"
#include "drawtypes/animation.hpp"
#include "drawtypes/label.hpp"
#include "drawtypes/progressbar.hpp"
#include "drawtypes/ramp.hpp"
#include "modules/meta/base.inl"
#include "utils/file.hpp"
#include "utils/math.hpp"
#include "utils/string.hpp"
#include "modules/meta/base.inl"
POLYBAR_NS
namespace modules {
@ -93,8 +93,8 @@ namespace modules {
unsigned long voltage{std::strtoul(file_util::contents(m_fvoltage).c_str(), nullptr, 10)};
consumption = ((voltage / 1000.0) * (current / 1000.0)) / 1e6;
// if it was power, just use as is
} else {
// if it was power, just use as is
unsigned long power{std::strtoul(file_util::contents(m_frate).c_str(), nullptr, 10)};
consumption = power / 1e6;
@ -209,15 +209,15 @@ namespace modules {
/**
* Update values when tracked files have changed
*/
bool battery_module::on_event(inotify_event* event) {
bool battery_module::on_event(const inotify_event& event) {
auto state = current_state();
auto percentage = current_percentage();
// Reset timer to avoid unnecessary polling
m_lastpoll = chrono::steady_clock::now();
if (event != nullptr) {
m_log.trace("%s: Inotify event reported for %s", name(), event->filename);
if (event.is_valid) {
m_log.trace("%s: Inotify event reported for %s", name(), event.filename);
if (state == m_state && percentage == m_percentage && m_unchanged--) {
return false;
@ -257,14 +257,17 @@ namespace modules {
*/
string battery_module::get_format() const {
switch (m_state) {
case battery_module::state::FULL: return FORMAT_FULL;
case battery_module::state::FULL:
return FORMAT_FULL;
case battery_module::state::LOW:
if (m_formatter->has_format(FORMAT_LOW)) {
return FORMAT_LOW;
}
return FORMAT_DISCHARGING;
case battery_module::state::DISCHARGING: return FORMAT_DISCHARGING;
default: return FORMAT_CHARGING;
case battery_module::state::DISCHARGING:
return FORMAT_DISCHARGING;
default:
return FORMAT_CHARGING;
}
}

View File

@ -14,8 +14,7 @@ namespace modules {
/**
* Construct module
*/
github_module::github_module(const bar_settings& bar, string name_)
: timer_module<github_module>(bar, move(name_)), m_http(http_util::make_downloader()) {
github_module::github_module(const bar_settings& bar, string name_) : timer_module<github_module>(bar, move(name_)) {
m_accesstoken = m_conf.get(name(), "token");
m_user = m_conf.get(name(), "user", ""s);
m_api_url = m_conf.get(name(), "api-url", "https://api.github.com/"s);
@ -55,12 +54,12 @@ namespace modules {
string github_module::request() {
string content;
if (m_user.empty()) {
content = m_http->get(m_api_url + "notifications?access_token=" + m_accesstoken);
content = m_http.get(m_api_url + "notifications?access_token=" + m_accesstoken);
} else {
content = m_http->get(m_api_url + "notifications", m_user, m_accesstoken);
content = m_http.get(m_api_url + "notifications", m_user, m_accesstoken);
}
long response_code{m_http->response_code()};
long response_code{m_http.response_code()};
switch (response_code) {
case 200:
break;

View File

@ -214,9 +214,9 @@ namespace modules {
m_output.clear();
try {
auto command = command_util::make_command<output_policy::REDIRECTED>(m_hooks[m_current_hook]->command);
command->exec(false);
command->tail([this](string line) { m_output = line; });
command<output_policy::REDIRECTED> cmd(m_log, m_hooks[m_current_hook]->command);
cmd.exec(false);
cmd.tail([this](string line) { m_output = line; });
} catch (const exception& err) {
m_log.err("%s: Failed to execute hook command (err: %s)", name(), err.what());
m_output.clear();

View File

@ -1,7 +0,0 @@
#include "utils/factory.hpp"
POLYBAR_NS
factory_util::detail::null_deleter factory_util::null_deleter{};
POLYBAR_NS_END

View File

@ -70,13 +70,15 @@ bool inotify_watch::poll(int wait_ms) const {
/**
* Get the latest inotify event
*/
unique_ptr<inotify_event> inotify_watch::get_event() const {
auto event = std::make_unique<inotify_event>();
inotify_event inotify_watch::get_event() const {
inotify_event event;
if (m_fd == -1 || m_wd == -1) {
return event;
}
event.is_valid = true;
char buffer[1024];
auto bytes = read(m_fd, buffer, 1024);
auto len = 0;
@ -84,11 +86,11 @@ unique_ptr<inotify_event> inotify_watch::get_event() const {
while (len < bytes) {
auto* e = reinterpret_cast<::inotify_event*>(&buffer[len]);
event->filename = e->len ? e->name : m_path;
event->wd = e->wd;
event->cookie = e->cookie;
event->is_dir = e->mask & IN_ISDIR;
event->mask |= e->mask;
event.filename = e->len ? e->name : m_path;
event.wd = e->wd;
event.cookie = e->cookie;
event.is_dir = e->mask & IN_ISDIR;
event.mask |= e->mask;
len += sizeof(*e) + e->len;
}
@ -96,14 +98,6 @@ unique_ptr<inotify_event> inotify_watch::get_event() const {
return event;
}
/**
* Wait for matching event
*/
unique_ptr<inotify_event> inotify_watch::await_match() const {
auto event = get_event();
return event->mask & m_mask ? std::move(event) : nullptr;
}
/**
* Get watch file path
*/

View File

@ -156,11 +156,12 @@ xcb_visualtype_t* connection::visual_type_for_id(xcb_screen_t* screen, xcb_visua
if (depth_iter.data) {
for (; depth_iter.rem; xcb_depth_next(&depth_iter)) {
for (auto it = xcb_depth_visuals_iterator(depth_iter.data); it.rem; xcb_visualtype_next(&it)) {
if (it.data->visual_id == visual_id)
if (it.data->visual_id == visual_id) {
return it.data;
}
}
}
}
return nullptr;
}

View File

@ -866,7 +866,7 @@ bool tray_manager::is_embedded(const xcb_window_t& win) const {
shared_ptr<tray_client> tray_manager::find_client(const xcb_window_t& win) const {
for (auto&& client : m_clients) {
if (client->match(win)) {
return shared_ptr<tray_client>{client.get(), factory_util::null_deleter};
return client;
}
}
return nullptr;

View File

@ -6,19 +6,21 @@
using namespace polybar;
static logger null_logger(loglevel::NONE);
TEST(Command, status) {
// Test for command<output_policy::IGNORED>::exec(bool);
{
auto cmd = command_util::make_command<output_policy::IGNORED>("echo polybar > /dev/null");
int status = cmd->exec();
command<output_policy::IGNORED> cmd(null_logger, "echo polybar > /dev/null");
int status = cmd.exec();
EXPECT_EQ(status, EXIT_SUCCESS);
}
// Test for command<output_policy::REDIRECTED>::exec(bool);
{
auto cmd = command_util::make_command<output_policy::REDIRECTED>("echo polybar");
int status = cmd->exec();
command<output_policy::REDIRECTED> cmd(null_logger, "echo polybar");
int status = cmd.exec();
EXPECT_EQ(status, EXIT_SUCCESS);
}
@ -26,22 +28,22 @@ TEST(Command, status) {
TEST(Command, status_async) {
{
auto cmd = command_util::make_command<output_policy::IGNORED>("echo polybar > /dev/null");
EXPECT_EQ(cmd->exec(false), EXIT_SUCCESS);
command<output_policy::IGNORED> cmd(null_logger, "echo polybar > /dev/null");
EXPECT_EQ(cmd.exec(false), EXIT_SUCCESS);
cmd->wait();
cmd.wait();
EXPECT_FALSE(cmd->is_running());
EXPECT_EQ(cmd->get_exit_status(), EXIT_SUCCESS);
EXPECT_FALSE(cmd.is_running());
EXPECT_EQ(cmd.get_exit_status(), EXIT_SUCCESS);
}
}
TEST(Command, output) {
auto cmd = command_util::make_command<output_policy::REDIRECTED>("echo polybar");
command<output_policy::REDIRECTED> cmd(null_logger, "echo polybar");
string str;
cmd->exec(false);
cmd->tail([&str](string&& string) { str = string; });
cmd->wait();
cmd.exec(false);
cmd.tail([&str](string&& string) { str = string; });
cmd.wait();
EXPECT_EQ(str, "polybar");
}

View File

@ -173,14 +173,3 @@ TEST(String, filesize) {
EXPECT_EQ("3 GB", string_util::filesize((unsigned long long)3 * 1024 * 1024 * 1024));
EXPECT_EQ("3 TB", string_util::filesize((unsigned long long)3 * 1024 * 1024 * 1024 * 1024));
}
TEST(String, operators) {
string foo = "foobar";
EXPECT_EQ("foo", foo - "bar");
string baz = "bazbaz";
EXPECT_EQ("bazbaz", baz - "ba");
EXPECT_EQ("bazbaz", baz - "baZ");
EXPECT_EQ("bazbaz", baz - "bazbz");
string aaa = "aaa";
EXPECT_EQ("aaa", aaa - "aaaaa");
}