Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor libstuff cleanup #2092

Merged
merged 8 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libstuff/AutoTimer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#undef SLOGPREFIX
#define SLOGPREFIX "{} "

AutoTimer::AutoTimer(string name) : _name(name), _intervalStart(chrono::steady_clock::now()), _countedTime(0) {
AutoTimer::AutoTimer(const string& name) : _name(name), _intervalStart(chrono::steady_clock::now()), _countedTime(0) {
}

void AutoTimer::start() {
Expand Down
2 changes: 1 addition & 1 deletion libstuff/AutoTimer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ using namespace std;
// There is a *different* AutoTimer in BedrockCore, which is annoying.
class AutoTimer {
public:
AutoTimer(string name);
AutoTimer(const string& name);
void start();
void stop();

Expand Down
2 changes: 1 addition & 1 deletion libstuff/SLog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,6 @@ string addLogParams(string&& message, const STable& params) {
return message;
}

void SWhitelistLogParams(set<string> params) {
void SWhitelistLogParams(const set<string>& params) {
PARAMS_WHITELIST.insert(params.begin(), params.end());
}
1 change: 0 additions & 1 deletion libstuff/SMultiHostSocketPool.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#pragma once
#include <libstuff/STCPManager.h>
#include <libstuff/SSocketPool.h>
#include <condition_variable>

class SMultiHostSocketPool {
public:
Expand Down
2 changes: 1 addition & 1 deletion libstuff/SPerformanceTimer.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <libstuff/libstuff.h>
#include "SPerformanceTimer.h"

SPerformanceTimer::SPerformanceTimer(string description, map<string, chrono::steady_clock::duration> defaults)
SPerformanceTimer::SPerformanceTimer(const string& description, const map<string, chrono::steady_clock::duration>& defaults)
: _description(description),
_lastLogStart(chrono::steady_clock::now()),
_defaults(defaults),
Expand Down
2 changes: 1 addition & 1 deletion libstuff/SPerformanceTimer.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

class SPerformanceTimer {
public:
SPerformanceTimer(string description, map<string, chrono::steady_clock::duration> defaults = {});
SPerformanceTimer(const string& description, const map<string, chrono::steady_clock::duration>& defaults = {});
void start(const string& type);
void stop();
void log(chrono::steady_clock::duration elapsed);
Expand Down
2 changes: 1 addition & 1 deletion libstuff/STCPManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ unique_ptr<STCPManager::Port> STCPManager::openPort(const string& host, int rema
return make_unique<Port>(s, host);
}

STCPManager::Port::Port(int _s, string _host) : s(_s), host(_host)
STCPManager::Port::Port(int _s, const string& _host) : s(_s), host(_host)
{
}

Expand Down
4 changes: 2 additions & 2 deletions libstuff/STCPManager.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once
#include <atomic>
#include <list>
#include <memory>
#include <mutex>
#include <netinet/in.h>
#include <poll.h>
Expand Down Expand Up @@ -64,7 +64,7 @@ struct STCPManager {

class Port {
public:
Port(int _s, string _host);
Port(int _s, const string& _host);
~Port();

// Attributes
Expand Down
4 changes: 3 additions & 1 deletion libstuff/libstuff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include <cxxabi.h>
#include <sys/ioctl.h>

#include <thread>

#include "libstuff.h"
#include <sys/stat.h>
#include <zlib.h>
Expand Down Expand Up @@ -102,7 +104,7 @@ atomic_flag SLogSocketsInitialized = ATOMIC_FLAG_INIT;
// Set to `syslog` or `SSyslogSocketDirect`.
atomic<void (*)(int priority, const char *format, ...)> SSyslogFunc = &syslog;

void SInitialize(string threadName, const char* processName) {
void SInitialize(const string& threadName, const char* processName) {
isSyncThread = false;
// This is not really thread safe. It's guaranteed to run only once, because of the atomic flag, but it's not
// guaranteed that a second caller to `SInitialize` will wait until this block has completed before attempting to
Expand Down
12 changes: 5 additions & 7 deletions libstuff/libstuff.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@
#include <map>
#include <mutex>
#include <set>
#include <shared_mutex>
#include <sstream>
#include <string>
#include <thread>
#include <vector>

// Forward declarations of types only used by reference.
Expand All @@ -34,7 +32,7 @@ extern atomic<bool> GLOBAL_IS_LIVE;
extern void* SSIGNAL_NOTIFY_INTERRUPT;

// Initialize libstuff on every thread before calling any of its functions
void SInitialize(string threadName = "", const char* processName = 0);
void SInitialize(const string& threadName = "", const char* processName = 0);

// This function sets a lambda that will be executed while the process is being killed for any reason
// (e.g. it crashed). Since we usually add logs in the lambda function, we'll also need to return the log as a
Expand Down Expand Up @@ -234,7 +232,7 @@ void SLogLevel(int level);
void SLogStackTrace(int level = LOG_WARNING);

// This method will allow plugins to whitelist log params they need to log.
void SWhitelistLogParams(set<string> params);
void SWhitelistLogParams(const set<string>& params);

// This is a drop-in replacement for syslog that directly logs to `/run/systemd/journal/syslog` bypassing journald.
void SSyslogSocketDirect(int priority, const char* format, ...);
Expand Down Expand Up @@ -309,15 +307,15 @@ struct SAutoThreadPrefix {
namespace std {
template<>
struct atomic<string> {
string operator=(string desired) {
string operator=(const string& desired) {
lock_guard<decltype(m)> l(m);
_string = desired;
return _string;
}
bool is_lock_free() const {
return false;
}
void store(string desired, [[maybe_unused]] std::memory_order order = std::memory_order_seq_cst) {
void store(const string& desired, [[maybe_unused]] std::memory_order order = std::memory_order_seq_cst) {
lock_guard<decltype(m)> l(m);
_string = desired;
};
Expand All @@ -329,7 +327,7 @@ namespace std {
lock_guard<decltype(m)> l(m);
return _string;
}
string exchange(string desired, [[maybe_unused]] std::memory_order order = std::memory_order_seq_cst) {
string exchange(const string& desired, [[maybe_unused]] std::memory_order order = std::memory_order_seq_cst) {
lock_guard<decltype(m)> l(m);
string existing = _string;
_string = desired;
Expand Down
3 changes: 1 addition & 2 deletions sqlitecluster/SQLite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#include <linux/limits.h>
#include <string.h>
#include <format>

#include <libstuff/libstuff.h>
#include <libstuff/SQResult.h>
Expand Down Expand Up @@ -912,7 +911,7 @@ bool SQLite::getCommit(uint64_t id, string& query, string& hash) {
return getCommit(_db, _journalNames, id, query, hash);
}

bool SQLite::getCommit(sqlite3* db, const vector<string> journalNames, uint64_t id, string& query, string& hash) {
bool SQLite::getCommit(sqlite3* db, const vector<string>& journalNames, uint64_t id, string& query, string& hash) {
// TODO: This can fail if called after `BEGIN TRANSACTION`, if the id we want to look up was committed by another
// thread. We may or may never need to handle this case.
// Look up the query and hash for the given commit
Expand Down
4 changes: 3 additions & 1 deletion sqlitecluster/SQLite.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <libstuff/SQResult.h>
#include <libstuff/SPerformanceTimer.h>

#include <shared_mutex>

class SQLite {
public:

Expand Down Expand Up @@ -223,7 +225,7 @@ class SQLite {
bool getCommit(uint64_t index, string& query, string& hash);

// A static version of the above that can be used in initializers.
static bool getCommit(sqlite3* db, const vector<string> journalNames, uint64_t index, string& query, string& hash);
static bool getCommit(sqlite3* db, const vector<string>& journalNames, uint64_t index, string& query, string& hash);

// Looks up a range of commits.
int getCommits(uint64_t fromIndex, uint64_t toIndex, SQResult& result, uint64_t timeoutLimitUS = 0);
Expand Down
4 changes: 2 additions & 2 deletions sqlitecluster/SQLiteClusterMessenger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <unistd.h>
#include <fcntl.h>

SQLiteClusterMessenger::SQLiteClusterMessenger(const shared_ptr<const SQLiteNode> node)
SQLiteClusterMessenger::SQLiteClusterMessenger(const shared_ptr<const SQLiteNode>& node)
: _node(node), _socketPool()
{ }

Expand Down Expand Up @@ -220,7 +220,7 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket,
return true;
}

unique_ptr<SHTTPSManager::Socket> SQLiteClusterMessenger::_getSocketForAddress(string address) {
unique_ptr<SHTTPSManager::Socket> SQLiteClusterMessenger::_getSocketForAddress(const string& address) {
unique_ptr<SHTTPSManager::Socket> s;

// SParseURI expects a typical http or https scheme.
Expand Down
4 changes: 2 additions & 2 deletions sqlitecluster/SQLiteClusterMessenger.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class SQLiteClusterMessenger {
POLL_ERROR,
};

SQLiteClusterMessenger(const shared_ptr<const SQLiteNode> node);
SQLiteClusterMessenger(const shared_ptr<const SQLiteNode>& node);

// Attempts to make a TCP connection to a peer, that could be the leader or not, and run the given command there,
// setting the appropriate response from the peer in the command, and marking it as complete if possible.
Expand Down Expand Up @@ -64,7 +64,7 @@ class SQLiteClusterMessenger {
// Parses the address to confirm it is valid, then requests a socket from
// the socket pool. Returns either a pointer to the socket or nullptr if
// there is an error.
unique_ptr<SHTTPSManager::Socket> _getSocketForAddress(string address);
unique_ptr<SHTTPSManager::Socket> _getSocketForAddress(const string& address);

const shared_ptr<const SQLiteNode> _node;

Expand Down
1 change: 1 addition & 0 deletions sqlitecluster/SQLiteServer.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once
class SQLiteCommand;
class SQLitePeer;
#include "SQLiteNode.h"

#include <libstuff/STCPManager.h>

Expand Down