Code review - general gardening

src/common/construct.h

@@ -6,28 +6,29 @@
 
 #include <boost/serialization/serialization.hpp>
 
+/// Allows classes to define `save_construct` and `load_construct` methods for serialization
+/// This is used where we don't call the default constructor during deserialization, as a shortcut
+/// instead of using load_construct_data directly
 class construct_access {
 public:
     template <class Archive, class T>
-    static inline void save_construct(Archive& ar, const T* t, const unsigned int file_version) {
+    static void save_construct(Archive& ar, const T* t, const unsigned int file_version) {
         t->save_construct(ar, file_version);
     }
     template <class Archive, class T>
-    static inline void load_construct(Archive& ar, T* t, const unsigned int file_version) {
+    static void load_construct(Archive& ar, T* t, const unsigned int file_version) {
         T::load_construct(ar, t, file_version);
     }
 };
 
 #define BOOST_SERIALIZATION_CONSTRUCT(T)                                                           \
-    namespace boost {                                                                              \
+    namespace boost::serialization {                                                               \
-    namespace serialization {                                                                      \
     template <class Archive>                                                                       \
-    inline void save_construct_data(Archive& ar, const T* t, const unsigned int file_version) {    \
+    void save_construct_data(Archive& ar, const T* t, const unsigned int file_version) {           \
         construct_access::save_construct(ar, t, file_version);                                     \
     }                                                                                              \
     template <class Archive>                                                                       \
-    inline void load_construct_data(Archive& ar, T* t, const unsigned int file_version) {          \
+    void load_construct_data(Archive& ar, T* t, const unsigned int file_version) {                 \
         construct_access::load_construct(ar, t, file_version);                                     \
     }                                                                                              \
-    }                                                                                              \
     }

src/common/memory_ref.h

@@ -17,7 +17,8 @@ class BackingMem {
 public:
     virtual ~BackingMem() = default;
     virtual u8* GetPtr() = 0;
-    virtual u32 GetSize() const = 0;
+    virtual const u8* GetPtr() const = 0;
+    virtual std::size_t GetSize() const = 0;
 
 private:
     template <class Archive>
@@ -35,7 +36,11 @@ public:
         return data.data();
     }
 
-    u32 GetSize() const override {
+    const u8* GetPtr() const override {
+        return data.data();
+    }
+
+    std::size_t GetSize() const override {
         return static_cast<u32>(data.size());
     }
 
@@ -66,51 +71,51 @@ public:
         : backing_mem(std::move(backing_mem_)), offset(0) {
         Init();
     }
-    MemoryRef(std::shared_ptr<BackingMem> backing_mem_, u32 offset_)
+    MemoryRef(std::shared_ptr<BackingMem> backing_mem_, u64 offset_)
         : backing_mem(std::move(backing_mem_)), offset(offset_) {
         ASSERT(offset < backing_mem->GetSize());
         Init();
     }
-    inline operator u8*() {
-        return cptr;
-    }
-    inline u8* GetPtr() {
-        return cptr;
-    }
     explicit operator bool() const {
         return cptr != nullptr;
     }
-    inline const u8* GetPtr() const {
+    operator u8*() {
         return cptr;
     }
-    inline u32 GetSize() const {
+    u8* GetPtr() {
+        return cptr;
+    }
+    operator const u8*() const {
+        return cptr;
+    }
+    const u8* GetPtr() const {
+        return cptr;
+    }
+    std::size_t GetSize() const {
         return csize;
     }
-    inline void operator+=(u32 offset_by) {
+    MemoryRef& operator+=(u32 offset_by) {
         ASSERT(offset_by < csize);
         offset += offset_by;
         Init();
+        return *this;
     }
-    inline MemoryRef operator+(u32 offset_by) const {
+    MemoryRef operator+(u32 offset_by) const {
         ASSERT(offset_by < csize);
         return MemoryRef(backing_mem, offset + offset_by);
     }
-    inline u8* operator+(std::size_t offset_by) const {
-        ASSERT(offset_by < csize);
-        return cptr + offset_by;
-    }
 
 private:
-    std::shared_ptr<BackingMem> backing_mem = nullptr;
+    std::shared_ptr<BackingMem> backing_mem{};
-    u32 offset = 0;
+    u64 offset{};
     // Cached values for speed
-    u8* cptr = nullptr;
+    u8* cptr{};
-    u32 csize = 0;
+    std::size_t csize{};
 
     void Init() {
         if (backing_mem) {
             cptr = backing_mem->GetPtr() + offset;
-            csize = static_cast<u32>(backing_mem->GetSize() - offset);
+            csize = static_cast<std::size_t>(backing_mem->GetSize() - offset);
         } else {
             cptr = nullptr;
             csize = 0;

src/common/thread_queue_list.h

@@ -161,7 +161,7 @@ private:
     // The priority level queues of thread ids.
     std::array<Queue, NUM_QUEUES> queues;
 
-    s32 ToIndex(Queue* q) const {
+    s64 ToIndex(const Queue* q) const {
         if (q == nullptr) {
             return -2;
         } else if (q == UnlinkedTag()) {
@@ -171,7 +171,7 @@ private:
         }
     }
 
-    Queue* ToPointer(s32 idx) {
+    Queue* ToPointer(s64 idx) {
         if (idx == -1) {
             return UnlinkedTag();
         } else if (idx < 0) {
@@ -184,10 +184,10 @@ private:
     friend class boost::serialization::access;
     template <class Archive>
     void save(Archive& ar, const unsigned int file_version) const {
-        s32 idx = ToIndex(first);
+        const s64 idx = ToIndex(first);
         ar << idx;
         for (size_t i = 0; i < NUM_QUEUES; i++) {
-            const s32 idx1 = ToIndex(queues[i].next_nonempty);
+            const s64 idx1 = ToIndex(queues[i].next_nonempty);
             ar << idx1;
             ar << queues[i].data;
         }
@@ -195,7 +195,7 @@ private:
 
     template <class Archive>
     void load(Archive& ar, const unsigned int file_version) {
-        s32 idx;
+        s64 idx;
         ar >> idx;
         first = ToPointer(idx);
         for (auto i = 0; i < NUM_QUEUES; i++) {

src/core/arm/arm_interface.h

@@ -36,11 +36,11 @@ public:
                 const auto r = GetFpuRegister(i);
                 ar << r;
             }
-            auto r1 = GetCpsr();
+            const auto r1 = GetCpsr();
             ar << r1;
-            auto r2 = GetFpscr();
+            const auto r2 = GetFpscr();
             ar << r2;
-            auto r3 = GetFpexc();
+            const auto r3 = GetFpexc();
             ar << r3;
         }
 
@@ -245,26 +245,26 @@ private:
     void save(Archive& ar, const unsigned int file_version) const {
         ar << timer;
         ar << id;
-        auto page_table = GetPageTable();
+        const auto page_table = GetPageTable();
         ar << page_table;
-        for (size_t i = 0; i < 15; i++) {
+        for (int i = 0; i < 15; i++) {
-            auto r = GetReg(i);
+            const auto r = GetReg(i);
             ar << r;
         }
-        auto pc = GetPC();
+        const auto pc = GetPC();
         ar << pc;
-        auto cpsr = GetCPSR();
+        const auto cpsr = GetCPSR();
         ar << cpsr;
-        for (size_t i = 0; i < 32; i++) {
+        for (int i = 0; i < 32; i++) {
-            auto r = GetVFPReg(i);
+            const auto r = GetVFPReg(i);
             ar << r;
         }
-        for (auto i = 0; i < VFPSystemRegister::VFP_SYSTEM_REGISTER_COUNT; i++) {
+        for (size_t i = 0; i < VFPSystemRegister::VFP_SYSTEM_REGISTER_COUNT; i++) {
-            auto r = GetVFPSystemReg(static_cast<VFPSystemRegister>(i));
+            const auto r = GetVFPSystemReg(static_cast<VFPSystemRegister>(i));
             ar << r;
         }
-        for (auto i = 0; i < CP15Register::CP15_REGISTER_COUNT; i++) {
+        for (size_t i = 0; i < CP15Register::CP15_REGISTER_COUNT; i++) {
-            auto r = GetCP15Register(static_cast<CP15Register>(i));
+            const auto r = GetCP15Register(static_cast<CP15Register>(i));
             ar << r;
         }
     }
@@ -274,11 +274,11 @@ private:
         PurgeState();
         ar >> timer;
         ar >> id;
-        std::shared_ptr<Memory::PageTable> page_table = nullptr;
+        std::shared_ptr<Memory::PageTable> page_table{};
         ar >> page_table;
         SetPageTable(page_table);
         u32 r;
-        for (size_t = 0; i < 15; i++) {
+        for (int i = 0; i < 15; i++) {
             ar >> r;
             SetReg(i, r);
         }
@@ -286,15 +286,15 @@ private:
         SetPC(r);
         ar >> r;
         SetCPSR(r);
-        for (auto i = 0; i < 32; i++) {
+        for (int i = 0; i < 32; i++) {
             ar >> r;
             SetVFPReg(i, r);
         }
-        for (auto i = 0; i < VFPSystemRegister::VFP_SYSTEM_REGISTER_COUNT; i++) {
+        for (size_t i = 0; i < VFPSystemRegister::VFP_SYSTEM_REGISTER_COUNT; i++) {
             ar >> r;
             SetVFPSystemReg(static_cast<VFPSystemRegister>(i), r);
         }
-        for (auto i = 0; i < CP15Register::CP15_REGISTER_COUNT; i++) {
+        for (size_t i = 0; i < CP15Register::CP15_REGISTER_COUNT; i++) {
             ar >> r;
             SetCP15Register(static_cast<CP15Register>(i), r);
         }

src/core/hle/kernel/config_mem.h

@@ -58,10 +58,14 @@ public:
     ConfigMemDef& GetConfigMem();
 
     u8* GetPtr() override {
-        return reinterpret_cast<u8*>(&config_mem));
+        return reinterpret_cast<u8*>(&config_mem);
     }
 
-    u32 GetSize() const override {
+    const u8* GetPtr() const override {
+        return reinterpret_cast<const u8*>(&config_mem);
+    }
+
+    std::size_t GetSize() const override {
         return sizeof(config_mem);
     }
 

src/core/hle/kernel/kernel.cpp

@@ -24,10 +24,8 @@ KernelSystem::KernelSystem(Memory::MemorySystem& memory, Core::Timing& timing,
                            u32 num_cores, u8 n3ds_mode)
     : memory(memory), timing(timing),
       prepare_reschedule_callback(std::move(prepare_reschedule_callback)) {
-std::generate(memory_regions.begin(), memory_regions.end(),
+    std::generate(memory_regions.begin(), memory_regions.end(),
                   [] { return std::make_shared<MemoryRegionInfo>(); });
-        memory_regions[i] = std::make_shared<MemoryRegionInfo>();
-    }
     MemoryInit(system_mode, n3ds_mode);
 
     resource_limits = std::make_unique<ResourceLimitList>(*this);

src/core/hle/kernel/shared_page.h

@@ -99,11 +99,15 @@ public:
 
     SharedPageDef& GetSharedPage();
 
-    virtual u8* GetPtr() {
+    u8* GetPtr() override {
-        return static_cast<u8*>(static_cast<void*>(&shared_page));
+        return reinterpret_cast<u8*>(&shared_page);
     }
 
-    virtual u32 GetSize() const {
+    const u8* GetPtr() const override {
+        return reinterpret_cast<const u8*>(&shared_page);
+    }
+
+    std::size_t GetSize() const override {
         return sizeof(shared_page);
     }
 

src/core/hle/service/cecd/cecd.cpp

@@ -106,7 +106,7 @@ void Module::Interface::Open(Kernel::HLERequestContext& ctx) {
         } else {
             session_data->file = std::move(file_result).Unwrap();
             rb.Push(RESULT_SUCCESS);
-            rb.Push<u32>(session_data->file->GetSize()); // Return file size
+            rb.Push<u32>(static_cast<u32>(session_data->file->GetSize())); // Return file size
         }
 
         if (path_type == CecDataPathType::MboxProgramId) {
@@ -154,8 +154,8 @@ void Module::Interface::Read(Kernel::HLERequestContext& ctx) {
         break;
     default: // If not directory, then it is a file
         std::vector<u8> buffer(write_buffer_size);
-        const u32 bytes_read =
+        const u32 bytes_read = static_cast<u32>(
-            session_data->file->Read(0, write_buffer_size, buffer.data()).Unwrap();
+            session_data->file->Read(0, write_buffer_size, buffer.data()).Unwrap());
 
         write_buffer.Write(buffer.data(), 0, write_buffer_size);
         session_data->file->Close();
@@ -197,7 +197,8 @@ void Module::Interface::ReadMessage(Kernel::HLERequestContext& ctx) {
         auto message = std::move(message_result).Unwrap();
         std::vector<u8> buffer(buffer_size);
 
-        const u32 bytes_read = message->Read(0, buffer_size, buffer.data()).Unwrap();
+        const u32 bytes_read =
+            static_cast<u32>(message->Read(0, buffer_size, buffer.data()).Unwrap());
         write_buffer.Write(buffer.data(), 0, buffer_size);
         message->Close();
 
@@ -266,7 +267,8 @@ void Module::Interface::ReadMessageWithHMAC(Kernel::HLERequestContext& ctx) {
         auto message = std::move(message_result).Unwrap();
         std::vector<u8> buffer(buffer_size);
 
-        const u32 bytes_read = message->Read(0, buffer_size, buffer.data()).Unwrap();
+        const u32 bytes_read =
+            static_cast<u32>(message->Read(0, buffer_size, buffer.data()).Unwrap());
         write_buffer.Write(buffer.data(), 0, buffer_size);
         message->Close();
 
@@ -367,8 +369,8 @@ void Module::Interface::Write(Kernel::HLERequestContext& ctx) {
                                      buffer);
         }
 
-        const u32 bytes_written =
+        const u32 bytes_written = static_cast<u32>(
-            session_data->file->Write(0, buffer.size(), true, buffer.data()).Unwrap();
+            session_data->file->Write(0, buffer.size(), true, buffer.data()).Unwrap());
         session_data->file->Close();
 
         rb.Push(RESULT_SUCCESS);
@@ -429,7 +431,8 @@ void Module::Interface::WriteMessage(Kernel::HLERequestContext& ctx) {
                   msg_header.sender_id, msg_header.sender_id2, msg_header.send_count,
                   msg_header.forward_count, msg_header.user_data);
 
-        const u32 bytes_written = message->Write(0, buffer_size, true, buffer.data()).Unwrap();
+        const u32 bytes_written =
+            static_cast<u32>(message->Write(0, buffer_size, true, buffer.data()).Unwrap());
         message->Close();
 
         rb.Push(RESULT_SUCCESS);
@@ -515,7 +518,8 @@ void Module::Interface::WriteMessageWithHMAC(Kernel::HLERequestContext& ctx) {
         hmac.CalculateDigest(hmac_digest.data(), message_body.data(), msg_header.body_size);
         std::memcpy(buffer.data() + hmac_offset, hmac_digest.data(), hmac_size);
 
-        const u32 bytes_written = message->Write(0, buffer_size, true, buffer.data()).Unwrap();
+        const u32 bytes_written =
+            static_cast<u32>(message->Write(0, buffer_size, true, buffer.data()).Unwrap());
         message->Close();
 
         rb.Push(RESULT_SUCCESS);
@@ -757,7 +761,8 @@ void Module::Interface::OpenAndWrite(Kernel::HLERequestContext& ctx) {
                 cecd->CheckAndUpdateFile(path_type, ncch_program_id, buffer);
             }
 
-            const u32 bytes_written = file->Write(0, buffer.size(), true, buffer.data()).Unwrap();
+            const u32 bytes_written =
+                static_cast<u32>(file->Write(0, buffer.size(), true, buffer.data()).Unwrap());
             file->Close();
 
             rb.Push(RESULT_SUCCESS);
@@ -806,7 +811,8 @@ void Module::Interface::OpenAndRead(Kernel::HLERequestContext& ctx) {
             auto file = std::move(file_result).Unwrap();
             std::vector<u8> buffer(buffer_size);
 
-            const u32 bytes_read = file->Read(0, buffer_size, buffer.data()).Unwrap();
+            const u32 bytes_read =
+                static_cast<u32>(file->Read(0, buffer_size, buffer.data()).Unwrap());
             write_buffer.Write(buffer.data(), 0, buffer_size);
             file->Close();
 
@@ -937,7 +943,7 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
     constexpr u32 max_num_boxes = 24;
     constexpr u32 name_size = 16;      // fixed size 16 characters long
     constexpr u32 valid_name_size = 8; // 8 characters are valid, the rest are null
-    const u32 file_size = file_buffer.size();
+    const u32 file_size = static_cast<u32>(file_buffer.size());
 
     switch (path_type) {
     case CecDataPathType::MboxList: {
@@ -1021,7 +1027,7 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
                 std::u16string u16_filename;
 
                 // Loop through entries but don't add mboxlist____ to itself.
-                for (auto i = 0; i < entry_count; i++) {
+                for (u32 i = 0; i < entry_count; i++) {
                     u16_filename = std::u16string(entries[i].filename);
                     file_name = Common::UTF16ToUTF8(u16_filename);
 
@@ -1212,7 +1218,7 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
         std::string file_name;
         std::u16string u16_filename;
 
-        for (auto i = 0; i < entry_count; i++) {
+        for (u32 i = 0; i < entry_count; i++) {
             u16_filename = std::u16string(entries[i].filename);
             file_name = Common::UTF16ToUTF8(u16_filename);
 
@@ -1230,7 +1236,7 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
                 auto message_result = cecd_system_save_data_archive->OpenFile(message_path, mode);
 
                 auto message = std::move(message_result).Unwrap();
-                const u32 message_size = message->GetSize();
+                const u32 message_size = static_cast<u32>(message->GetSize());
                 std::vector<u8> buffer(message_size);
 
                 message->Read(0, message_size, buffer.data()).Unwrap();
@@ -1304,7 +1310,7 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
         std::string file_name;
         std::u16string u16_filename;
 
-        for (auto i = 0; i < entry_count; i++) {
+        for (u32 i = 0; i < entry_count; i++) {
             u16_filename = std::u16string(entries[i].filename);
             file_name = Common::UTF16ToUTF8(u16_filename);
 
@@ -1320,7 +1326,7 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
                 auto message_result = cecd_system_save_data_archive->OpenFile(message_path, mode);
 
                 auto message = std::move(message_result).Unwrap();
-                const u32 message_size = message->GetSize();
+                const u32 message_size = static_cast<u32>(message->GetSize());
                 std::vector<u8> buffer(message_size);
 
                 message->Read(0, message_size, buffer.data()).Unwrap();
@@ -1353,7 +1359,8 @@ void Module::CheckAndUpdateFile(const CecDataPathType path_type, const u32 ncch_
     case CecDataPathType::MboxData:
     case CecDataPathType::MboxIcon:
     case CecDataPathType::MboxTitle:
-    default: {}
+    default: {
+    }
     }
 }
 

src/core/hle/service/http_c.cpp

@@ -83,10 +83,10 @@ void Context::MakeRequest() {
         client = std::move(ssl_client);
 
         if (auto client_cert = ssl_config.client_cert_ctx.lock()) {
-            SSL_CTX_use_certificate_ASN1(ctx, client_cert->certificate.size(),
+            SSL_CTX_use_certificate_ASN1(ctx, static_cast<int>(client_cert->certificate.size()),
                                          client_cert->certificate.data());
             SSL_CTX_use_PrivateKey_ASN1(EVP_PKEY_RSA, ctx, client_cert->private_key.data(),
-                                        client_cert->private_key.size());
+                                        static_cast<long>(client_cert->private_key.size()));
         }
 
         // TODO(B3N30): Check for SSLOptions-Bits and set the verify method accordingly

src/core/memory.cpp

@@ -100,7 +100,7 @@ public:
 
     Impl();
 
-    virtual u8* GetPtr(Region r) {
+    const u8* GetPtr(Region r) const {
         switch (r) {
         case Region::VRAM:
             return vram.get();
@@ -115,7 +115,22 @@ public:
         }
     }
 
-    virtual u32 GetSize(Region r) const {
+    u8* GetPtr(Region r) {
+        switch (r) {
+        case Region::VRAM:
+            return vram.get();
+        case Region::DSP:
+            return dsp->GetDspMemory().data();
+        case Region::FCRAM:
+            return fcram.get();
+        case Region::N3DS:
+            return n3ds_extra_ram.get();
+        default:
+            UNREACHABLE();
+        }
+    }
+
+    u32 GetSize(Region r) const {
         switch (r) {
         case Region::VRAM:
             return VRAM_SIZE;
@@ -158,11 +173,14 @@ template <Region R>
 class MemorySystem::BackingMemImpl : public BackingMem {
 public:
     BackingMemImpl() : impl(*Core::Global<Core::System>().Memory().impl) {}
-    BackingMemImpl(MemorySystem::Impl& impl_) : impl(impl_) {}
+    explicit BackingMemImpl(MemorySystem::Impl& impl_) : impl(impl_) {}
-    virtual u8* GetPtr() {
+    u8* GetPtr() override {
         return impl.GetPtr(R);
     }
-    virtual u32 GetSize() const {
+    const u8* GetPtr() const override {
+        return impl.GetPtr(R);
+    }
+    std::size_t GetSize() const override {
         return impl.GetSize(R);
     }
 
@@ -884,7 +902,7 @@ void WriteMMIO<u64>(MMIORegionPointer mmio_handler, VAddr addr, const u64 data)
 
 u32 MemorySystem::GetFCRAMOffset(const u8* pointer) {
     ASSERT(pointer >= impl->fcram.get() && pointer <= impl->fcram.get() + Memory::FCRAM_N3DS_SIZE);
-    return pointer - impl->fcram.get();
+    return static_cast<u32>(pointer - impl->fcram.get());
 }
 
 u8* MemorySystem::GetFCRAMPointer(u32 offset) {