Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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 example/calculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ int main(int argc, char** argv)
return 1;
}
mp::EventLoop loop("mpcalculator", LogPrint);
int fd = std::stoi(argv[1]);
const int fd = std::stoi(argv[1]);
std::unique_ptr<Init> init = std::make_unique<InitImpl>();
mp::ServeStream<InitInterface>(loop, fd, *init);
loop.loop();
Expand Down
2 changes: 1 addition & 1 deletion example/example.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace fs = std::filesystem;
auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const std::string& new_exe_name)
{
int pid;
int fd = mp::SpawnProcess(pid, [&](int fd) -> std::vector<std::string> {
const int fd = mp::SpawnProcess(pid, [&](int fd) -> std::vector<std::string> {
fs::path path = process_argv0;
path.remove_filename();
path.append(new_exe_name);
Expand Down
2 changes: 1 addition & 1 deletion example/printer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ int main(int argc, char** argv)
return 1;
}
mp::EventLoop loop("mpprinter", LogPrint);
int fd = std::stoi(argv[1]);
const int fd = std::stoi(argv[1]);
std::unique_ptr<Init> init = std::make_unique<InitImpl>();
mp::ServeStream<InitInterface>(loop, fd, *init);
loop.loop();
Expand Down
4 changes: 2 additions & 2 deletions include/mp/proxy-io.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ struct Waiter
template <typename Fn>
void post(Fn&& fn)
{
std::unique_lock<std::mutex> lock(m_mutex);
std::unique_lock<std::mutex> lock(m_mutex); // NOLINT(misc-const-correctness)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "clang-tidy: Fix misc-const-correctness check" (06806fe)

Maybe just declare these const? It seems more stable than adding NOLINT. Especially because if any an unlock call is made to any of these locks later the lint errors would go away but nolint comments would still be there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: #144 (comment)

This is done in #152

assert(!m_fn);
m_fn = std::move(fn);
m_cv.notify_all();
Expand All @@ -269,7 +269,7 @@ struct Waiter
fn();
lock.lock();
}
bool done = pred();
const bool done = pred();
return done;
});
}
Expand Down
4 changes: 2 additions & 2 deletions include/mp/proxy-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -626,15 +626,15 @@ void clientInvoke(ProxyClient& proxy_client, const GetRequest& get_request, Fiel
} catch (...) {
exception = std::current_exception();
}
std::unique_lock<std::mutex> lock(invoke_context.thread_context.waiter->m_mutex);
std::unique_lock<std::mutex> lock(invoke_context.thread_context.waiter->m_mutex); // NOLINT(misc-const-correctness)
done = true;
invoke_context.thread_context.waiter->m_cv.notify_all();
},
[&](const ::kj::Exception& e) {
kj_exception = kj::str("kj::Exception: ", e).cStr();
proxy_client.m_context.connection->m_loop.logPlain()
<< "{" << invoke_context.thread_context.thread_name << "} IPC client exception " << kj_exception;
std::unique_lock<std::mutex> lock(invoke_context.thread_context.waiter->m_mutex);
std::unique_lock<std::mutex> lock(invoke_context.thread_context.waiter->m_mutex); // NOLINT(misc-const-correctness)
done = true;
invoke_context.thread_context.waiter->m_cv.notify_all();
}));
Expand Down
2 changes: 1 addition & 1 deletion include/mp/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ struct UnlockGuard
template <typename Lock, typename Callback>
void Unlock(Lock& lock, Callback&& callback)
{
UnlockGuard<Lock> unlock(lock);
UnlockGuard<Lock> unlock(lock); // NOLINT(misc-const-correctness)
callback();
}

Expand Down
28 changes: 14 additions & 14 deletions src/mp/gen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void Generate(kj::StringPtr src_prefix,
}

std::string include_base = include_path;
std::string::size_type p = include_base.rfind('.');
const std::string::size_type p = include_base.rfind('.');
if (p != std::string::npos) include_base.erase(p);

std::vector<std::string> args;
Expand All @@ -165,19 +165,19 @@ void Generate(kj::StringPtr src_prefix,
}
args.emplace_back("--output=" capnp_PREFIX "/bin/capnpc-c++");
args.emplace_back(src_file);
int pid = fork();
const int pid = fork();
if (pid == -1) {
throw std::system_error(errno, std::system_category(), "fork");
}
if (!pid) {
mp::ExecProcess(args);
}
int status = mp::WaitProcess(pid);
const int status = mp::WaitProcess(pid);
if (status) {
throw std::runtime_error("Invoking " capnp_PREFIX "/bin/capnp failed");
}

capnp::SchemaParser parser;
const capnp::SchemaParser parser;
auto directory_pointers = kj::heapArray<const kj::ReadableDirectory*>(import_dirs.size());
for (size_t i = 0; i < import_dirs.size(); ++i) {
directory_pointers[i] = import_dirs[i].get();
Expand Down Expand Up @@ -244,7 +244,7 @@ void Generate(kj::StringPtr src_prefix,
GetAnnotationText(file_schema.getProto(), NAMESPACE_ANNOTATION_ID, &message_namespace);

std::string base_name = include_base;
size_t output_slash = base_name.rfind('/');
const size_t output_slash = base_name.rfind('/');
if (output_slash != std::string::npos) {
base_name.erase(0, output_slash + 1);
}
Expand All @@ -260,7 +260,7 @@ void Generate(kj::StringPtr src_prefix,

auto add_accessor = [&](kj::StringPtr name) {
if (!accessors_done.insert(name).second) return;
std::string cap = Cap(name);
const std::string cap = Cap(name);
accessors << "struct " << cap << "\n";
accessors << "{\n";
accessors << " template<typename S> static auto get(S&& s) -> decltype(s.get" << cap << "()) { return s.get" << cap << "(); }\n";
Expand Down Expand Up @@ -368,19 +368,19 @@ void Generate(kj::StringPtr src_prefix,
server << " using ProxyServerCustom::ProxyServerCustom;\n";
server << " ~ProxyServer();\n";

std::ostringstream client_construct;
std::ostringstream client_destroy;
const std::ostringstream client_construct;
const std::ostringstream client_destroy;

int method_ordinal = 0;
ForEachMethod(interface, [&] (const capnp::InterfaceSchema& method_interface, const capnp::InterfaceSchema::Method& method) {
kj::StringPtr method_name = method.getProto().getName();
const kj::StringPtr method_name = method.getProto().getName();
kj::StringPtr proxied_method_name = method_name;
GetAnnotationText(method.getProto(), NAME_ANNOTATION_ID, &proxied_method_name);

const std::string method_prefix = Format() << message_namespace << "::" << method_interface.getShortDisplayName()
<< "::" << Cap(method_name);
bool is_construct = method_name == "construct";
bool is_destroy = method_name == "destroy";
const bool is_construct = method_name == "construct";
const bool is_destroy = method_name == "destroy";

struct Field
{
Expand Down Expand Up @@ -530,9 +530,9 @@ void Generate(kj::StringPtr src_prefix,
server_invoke_end << ")";
}

std::string static_str{is_construct || is_destroy ? "static " : ""};
std::string super_str{is_construct || is_destroy ? "Super& super" : ""};
std::string self_str{is_construct || is_destroy ? "super" : "*this"};
const std::string static_str{is_construct || is_destroy ? "static " : ""};
const std::string super_str{is_construct || is_destroy ? "Super& super" : ""};
const std::string self_str{is_construct || is_destroy ? "super" : "*this"};

client << " using M" << method_ordinal << " = ProxyClientMethodTraits<" << method_prefix
<< "Params>;\n";
Expand Down
24 changes: 12 additions & 12 deletions src/mp/proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ Connection::~Connection()
m_sync_cleanup_fns.pop_front();
}
while (!m_async_cleanup_fns.empty()) {
std::unique_lock<std::mutex> lock(m_loop.m_mutex);
std::unique_lock<std::mutex> lock(m_loop.m_mutex); // NOLINT(misc-const-correctness)
m_loop.m_async_fns.emplace_back(std::move(m_async_cleanup_fns.front()));
m_async_cleanup_fns.pop_front();
}
Expand All @@ -113,7 +113,7 @@ Connection::~Connection()

CleanupIt Connection::addSyncCleanup(std::function<void()> fn)
{
std::unique_lock<std::mutex> lock(m_loop.m_mutex);
std::unique_lock<std::mutex> lock(m_loop.m_mutex); // NOLINT(misc-const-correctness)
// Add cleanup callbacks to the front of list, so sync cleanup functions run
// in LIFO order. This is a good approach because sync cleanup functions are
// added as client objects are created, and it is natural to clean up
Expand All @@ -127,13 +127,13 @@ CleanupIt Connection::addSyncCleanup(std::function<void()> fn)

void Connection::removeSyncCleanup(CleanupIt it)
{
std::unique_lock<std::mutex> lock(m_loop.m_mutex);
std::unique_lock<std::mutex> lock(m_loop.m_mutex); // NOLINT(misc-const-correctness)
m_sync_cleanup_fns.erase(it);
}

void Connection::addAsyncCleanup(std::function<void()> fn)
{
std::unique_lock<std::mutex> lock(m_loop.m_mutex);
std::unique_lock<std::mutex> lock(m_loop.m_mutex); // NOLINT(misc-const-correctness)
// Add async cleanup callbacks to the back of the list. Unlike the sync
// cleanup list, this list order is more significant because it determines
// the order server objects are destroyed when there is a sudden disconnect,
Expand Down Expand Up @@ -169,7 +169,7 @@ EventLoop::EventLoop(const char* exe_name, LogFn log_fn, void* context)
EventLoop::~EventLoop()
{
if (m_async_thread.joinable()) m_async_thread.join();
std::lock_guard<std::mutex> lock(m_mutex);
std::lock_guard<std::mutex> lock(m_mutex); // NOLINT(misc-const-correctness)
KJ_ASSERT(m_post_fn == nullptr);
KJ_ASSERT(m_async_fns.empty());
KJ_ASSERT(m_wait_fd == -1);
Expand All @@ -192,7 +192,7 @@ void EventLoop::loop()
int post_fd{m_post_fd};
char buffer = 0;
for (;;) {
size_t read_bytes = wait_stream->read(&buffer, 0, 1).wait(m_io_context.waitScope);
const size_t read_bytes = wait_stream->read(&buffer, 0, 1).wait(m_io_context.waitScope);
if (read_bytes != 1) throw std::logic_error("EventLoop wait_stream closed unexpectedly");
std::unique_lock<std::mutex> lock(m_mutex);
if (m_post_fn) {
Expand All @@ -212,7 +212,7 @@ void EventLoop::loop()
log() << "EventLoop::loop bye.";
wait_stream = nullptr;
KJ_SYSCALL(::close(post_fd));
std::unique_lock<std::mutex> lock(m_mutex);
std::unique_lock<std::mutex> lock(m_mutex); // NOLINT(misc-const-correctness)
m_wait_fd = -1;
m_post_fd = -1;
}
Expand Down Expand Up @@ -258,7 +258,7 @@ void EventLoop::startAsyncThread(std::unique_lock<std::mutex>& lock)
while (true) {
if (!m_async_fns.empty()) {
addClient(lock);
std::function<void()> fn = std::move(m_async_fns.front());
const std::function<void()> fn = std::move(m_async_fns.front());
m_async_fns.pop_front();
Unlock(lock, fn);
removeClient(lock);
Expand All @@ -282,7 +282,7 @@ bool EventLoop::done(std::unique_lock<std::mutex>& lock)

std::tuple<ConnThread, bool> SetThread(ConnThreads& threads, std::mutex& mutex, Connection* connection, std::function<Thread::Client()> make_thread)
{
std::unique_lock<std::mutex> lock(mutex);
std::unique_lock<std::mutex> lock(mutex); // NOLINT(misc-const-correctness)
auto thread = threads.find(connection);
if (thread != threads.end()) return {thread, false};
thread = threads.emplace(
Expand All @@ -299,7 +299,7 @@ std::tuple<ConnThread, bool> SetThread(ConnThreads& threads, std::mutex& mutex,
// try unregister this callback after connection is destroyed.
thread->second.m_cleanup_it.reset();
// Remove connection pointer about to be destroyed from the map
std::unique_lock<std::mutex> lock(mutex);
std::unique_lock<std::mutex> lock(mutex); // NOLINT(misc-const-correctness)
threads.erase(thread);
});
return {thread, true};
Expand Down Expand Up @@ -339,7 +339,7 @@ ProxyServer<Thread>::~ProxyServer()
assert(m_thread_context.waiter.get());
std::unique_ptr<Waiter> waiter;
{
std::unique_lock<std::mutex> lock(m_thread_context.waiter->m_mutex);
std::unique_lock<std::mutex> lock(m_thread_context.waiter->m_mutex); // NOLINT(misc-const-correctness)
//! Reset thread context waiter pointer, as shutdown signal for done
//! lambda passed as waiter->wait() argument in makeThread code below.
waiter = std::move(m_thread_context.waiter);
Expand Down Expand Up @@ -367,7 +367,7 @@ ProxyServer<ThreadMap>::ProxyServer(Connection& connection) : m_connection(conne

kj::Promise<void> ProxyServer<ThreadMap>::makeThread(MakeThreadContext context)
{
std::string from = context.getParams().getName();
const std::string from = context.getParams().getName();
std::promise<ThreadContext*> thread_context;
std::thread thread([&thread_context, from, this]() {
g_thread_context.thread_name = ThreadName(m_connection.m_loop.m_exe_name) + " (from " + from + ")";
Expand Down
4 changes: 2 additions & 2 deletions src/mp/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ std::string LogEscape(const kj::StringTree& string)
std::string result;
string.visit([&](const kj::ArrayPtr<const char>& piece) {
if (result.size() > MAX_SIZE) return;
for (char c : piece) {
for (const char c : piece) {
if (c == '\\') {
result.append("\\\\");
} else if (c < 0x20 || c > 0x7e) {
Expand Down Expand Up @@ -116,7 +116,7 @@ int SpawnProcess(int& pid, FdToArgsFn&& fd_to_args)
}
if (!pid) {
// Child process must close all potentially open descriptors, except socket 0.
int maxFd = MaxFd();
const int maxFd = MaxFd();
for (int fd = 3; fd < maxFd; ++fd) {
if (fd != fds[0]) {
close(fd);
Expand Down