From d2c77e1d18d7814993215c2e09e77614f46c6c7b Mon Sep 17 00:00:00 2001 From: Drew Galbraith Date: Thu, 2 Nov 2023 23:51:52 -0700 Subject: [PATCH] [Zion] Move away from storing pointers to IpcMessages. --- lib/glacier/container/array.h | 2 +- lib/glacier/container/linked_list.h | 38 ++++++++++++++++++--------- zion/lib/message_queue.cpp | 40 ++++++++++++++--------------- zion/lib/message_queue.h | 2 +- 4 files changed, 48 insertions(+), 34 deletions(-) diff --git a/lib/glacier/container/array.h b/lib/glacier/container/array.h index b093ee6..2280a06 100644 --- a/lib/glacier/container/array.h +++ b/lib/glacier/container/array.h @@ -21,7 +21,7 @@ class Array { Array(const Array&) = delete; - Array(Array&& other) : data_(other.data), size_(other.size_) { + Array(Array&& other) : data_(other.data_), size_(other.size_) { other.data_ = nullptr; other.size_ = 0; } diff --git a/lib/glacier/container/linked_list.h b/lib/glacier/container/linked_list.h index b487c90..ad420ef 100644 --- a/lib/glacier/container/linked_list.h +++ b/lib/glacier/container/linked_list.h @@ -2,6 +2,8 @@ #include +#include "glacier/memory/move.h" + namespace glcr { template @@ -15,20 +17,19 @@ class LinkedList { uint64_t size() const { return size_; } void PushBack(const T& item) { - size_++; ListItem* new_item = new ListItem{ .item = item, .next = nullptr, }; - if (front_ == nullptr) { - front_ = new_item; - return; - } - ListItem* litem = front_; - while (litem->next != nullptr) { - litem = litem->next; - } - litem->next = new_item; + PushBackInternal(new_item); + } + + void PushBack(T&& item) { + ListItem* new_item = new ListItem{ + .item = glcr::Move(item), + .next = nullptr, + }; + PushBackInternal(new_item); } T PopFront() { @@ -36,12 +37,12 @@ class LinkedList { ListItem* old_front = front_; front_ = front_->next; - T ret = old_front->item; + T ret = glcr::Move(old_front->item); delete old_front; return ret; } - T PeekFront() const { return front_->item; } + T& PeekFront() const { return front_->item; } struct ListItem { T item; @@ -73,6 +74,19 @@ class LinkedList { uint64_t size_ = 0; ListItem* front_ = nullptr; + + void PushBackInternal(ListItem* new_item) { + size_++; + if (front_ == nullptr) { + front_ = new_item; + return; + } + ListItem* litem = front_; + while (litem->next != nullptr) { + litem = litem->next; + } + litem->next = new_item; + } }; } // namespace glcr diff --git a/zion/lib/message_queue.cpp b/zion/lib/message_queue.cpp index 813c8f5..0d09552 100644 --- a/zion/lib/message_queue.cpp +++ b/zion/lib/message_queue.cpp @@ -11,16 +11,16 @@ glcr::ErrorCode UnboundedMessageQueue::PushBack( return glcr::UNIMPLEMENTED; } - auto msg_struct = glcr::MakeShared(); - msg_struct->data = glcr::Array(message); + IpcMessage msg_struct; + msg_struct.data = glcr::Array(message); if (reply_cap != kZionInvalidCapability) { // FIXME: We're just trusting that capability has the correct permissions. - msg_struct->reply_cap = + msg_struct.reply_cap = gScheduler->CurrentProcess().ReleaseCapability(reply_cap); } - msg_struct->caps.Resize(caps.size()); + msg_struct.caps.Resize(caps.size()); for (uint64_t i = 0; i < caps.size(); i++) { // FIXME: This would feel safer closer to the relevant syscall. // FIXME: Race conditions on get->check->release here. Would be better to @@ -34,11 +34,11 @@ glcr::ErrorCode UnboundedMessageQueue::PushBack( return glcr::CAP_PERMISSION_DENIED; } cap = gScheduler->CurrentProcess().ReleaseCapability(caps[i]); - msg_struct->caps.PushBack(cap); + msg_struct.caps.PushBack(cap); } MutexHolder h(mutex_); - pending_messages_.PushBack(msg_struct); + pending_messages_.PushBack(glcr::Move(msg_struct)); if (blocked_threads_.size() > 0) { auto thread = blocked_threads_.PopFront(); @@ -64,34 +64,34 @@ glcr::ErrorCode UnboundedMessageQueue::PopFront(uint64_t* num_bytes, mutex_->Release(); MutexHolder lock(mutex_); - auto next_msg = pending_messages_.PeekFront(); - if (next_msg->data.size() > *num_bytes) { + auto& next_msg = pending_messages_.PeekFront(); + if (next_msg.data.size() > *num_bytes) { return glcr::BUFFER_SIZE; } - if (next_msg->caps.size() > *num_caps) { + if (next_msg.caps.size() > *num_caps) { return glcr::BUFFER_SIZE; } next_msg = pending_messages_.PopFront(); - *num_bytes = next_msg->data.size(); + *num_bytes = next_msg.data.size(); for (uint64_t i = 0; i < *num_bytes; i++) { - static_cast(bytes)[i] = next_msg->data[i]; + static_cast(bytes)[i] = next_msg.data[i]; } auto& proc = gScheduler->CurrentProcess(); if (reply_cap != nullptr) { - if (!next_msg->reply_cap) { + if (!next_msg.reply_cap) { dbgln("Tried to read reply capability off of a message without one"); return glcr::INTERNAL; } - *reply_cap = proc.AddExistingCapability(next_msg->reply_cap); + *reply_cap = proc.AddExistingCapability(next_msg.reply_cap); } - *num_caps = next_msg->caps.size(); + *num_caps = next_msg.caps.size(); for (uint64_t i = 0; i < *num_caps; i++) { - caps[i] = proc.AddExistingCapability(next_msg->caps[i]); + caps[i] = proc.AddExistingCapability(next_msg.caps[i]); } return glcr::OK; } @@ -99,16 +99,16 @@ glcr::ErrorCode UnboundedMessageQueue::PopFront(uint64_t* num_bytes, void UnboundedMessageQueue::WriteKernel(uint64_t init, glcr::RefPtr cap) { // FIXME: Add synchronization here in case it is ever used outside of init. - auto msg = glcr::MakeShared(); - msg->data = glcr::Array(sizeof(init)); + IpcMessage msg; + msg.data = glcr::Array(sizeof(init)); uint8_t* data = reinterpret_cast(&init); for (uint8_t i = 0; i < sizeof(init); i++) { - msg->data[i] = data[i]; + msg.data[i] = data[i]; } - msg->caps.PushBack(cap); + msg.caps.PushBack(cap); - pending_messages_.PushBack(msg); + pending_messages_.PushBack(glcr::Move(msg)); } glcr::ErrorCode SingleMessageQueue::PushBack( diff --git a/zion/lib/message_queue.h b/zion/lib/message_queue.h index 8210861..39cf906 100644 --- a/zion/lib/message_queue.h +++ b/zion/lib/message_queue.h @@ -61,7 +61,7 @@ class UnboundedMessageQueue : public MessageQueue { } private: - glcr::LinkedList> pending_messages_; + glcr::LinkedList pending_messages_; }; class SingleMessageQueue : public MessageQueue {