Tidying

src/DebugServer/Gdb/AvrGdb/CommandPackets/ReadMemory.cpp

@@ -22,7 +22,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
             throw Exception("Invalid packet length");
         }
 
-        auto packetString = QString::fromLocal8Bit(
+        const auto packetString = QString::fromLocal8Bit(
             reinterpret_cast<const char*>(this->data.data() + 1),
             static_cast<int>(this->data.size() - 1)
         );
@@ -31,7 +31,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
          * The read memory ('m') packet consists of two segments, an address and a number of bytes to read.
          * These are separated by a comma character.
          */
-        auto packetSegments = packetString.split(",");
+        const auto packetSegments = packetString.split(",");
 
         if (packetSegments.size() != 2) {
             throw Exception(
@@ -46,6 +46,10 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
             throw Exception("Failed to parse start address from read memory packet data");
         }
 
+        /*
+         * Extract the memory type from the memory address (see Gdb::TargetDescriptor::memoryOffsetsByType for more on
+         * this).
+         */
         this->memoryType = gdbTargetDescriptor.getMemoryTypeFromGdbAddress(gdbStartAddress);
         this->startAddress = gdbStartAddress & ~(gdbTargetDescriptor.getMemoryOffset(this->memoryType));
 
@@ -67,9 +71,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
             }
 
             if (this->bytes == 0) {
-                debugSession.connection.writePacket(
+                debugSession.connection.writePacket(ResponsePacket(std::vector<unsigned char>()));
-                    ResponsePacket(std::vector<unsigned char>())
-                );
                 return;
             }
 
@@ -112,6 +114,12 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
                 return;
             }
 
+            /*
+             * GDB may request more bytes than what's available (even though we give it a memory map?!) - ensure that
+             * we don't try to read any more than what's available.
+             *
+             * We fill the out-of-bounds bytes with 0x00, below.
+             */
             const auto bytesToRead = (this->startAddress <= memoryDescriptor.addressRange.endAddress)
                 ? std::min(this->bytes, (memoryDescriptor.addressRange.endAddress - this->startAddress) + 1)
                 : 0;
@@ -127,7 +135,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
             }
 
             if (bytesToRead < this->bytes) {
-                // GDB requested some out-of-bounds memory - fill the inaccessible bytes with 0s
+                // GDB requested some out-of-bounds memory - fill the inaccessible bytes with 0x00
                 memoryBuffer.insert(memoryBuffer.end(), (this->bytes - bytesToRead), 0x00);
             }
 

src/DebugServer/Gdb/AvrGdb/CommandPackets/WriteMemory.cpp

@@ -22,7 +22,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
             throw Exception("Invalid packet length");
         }
 
-        auto packetString = QString::fromLocal8Bit(
+        const auto packetString = QString::fromLocal8Bit(
             reinterpret_cast<const char*>(this->data.data() + 1),
             static_cast<int>(this->data.size() - 1)
         );
@@ -31,7 +31,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
          * The write memory ('M') packet consists of three segments, an address, a length and a buffer.
          * The address and length are separated by a comma character, and the buffer proceeds a colon character.
          */
-        auto packetSegments = packetString.split(",");
+        const auto packetSegments = packetString.split(",");
         if (packetSegments.size() != 2) {
             throw Exception(
                 "Unexpected number of segments in packet data: " + std::to_string(packetSegments.size())
@@ -48,7 +48,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
         this->memoryType = gdbTargetDescriptor.getMemoryTypeFromGdbAddress(gdbStartAddress);
         this->startAddress = gdbStartAddress & ~(gdbTargetDescriptor.getMemoryOffset(this->memoryType));
 
-        auto lengthAndBufferSegments = packetSegments.at(1).split(":");
+        const auto lengthAndBufferSegments = packetSegments.at(1).split(":");
         if (lengthAndBufferSegments.size() != 2) {
             throw Exception(
                 "Unexpected number of segments in packet data: "
@@ -56,7 +56,7 @@ namespace Bloom::DebugServer::Gdb::AvrGdb::CommandPackets
             );
         }
 
-        auto bufferSize = lengthAndBufferSegments.at(0).toUInt(&conversionStatus, 16);
+        const auto bufferSize = lengthAndBufferSegments.at(0).toUInt(&conversionStatus, 16);
         if (!conversionStatus) {
             throw Exception("Failed to parse write length from write memory packet data");
         }

src/DebugServer/Gdb/TargetDescriptor.hpp

@@ -37,6 +37,12 @@ namespace Bloom::DebugServer::Gdb
             return this->memoryOffsetsByType.valueAt(memoryType).value_or(0);
         }
 
+        /**
+         * Helper method to extract the target memory type (Flash, RAM, etc) from a GDB memory address.
+         *
+         * @param address
+         * @return
+         */
         Targets::TargetMemoryType getMemoryTypeFromGdbAddress(std::uint32_t address) const {
             // Start with the largest offset until we find a match
             for (
@@ -89,7 +95,19 @@ namespace Bloom::DebugServer::Gdb
         virtual const std::vector<GdbRegisterNumber>& getRegisterNumbers() const = 0;
 
     private:
+        /**
+         * When GDB sends us a memory address, the memory type (Flash, RAM, EEPROM, etc) is embedded within. This is
+         * done by ORing the address with some constant. For example, for AVR targets, RAM addresses are ORed with
+         * 0x00800000. Flash addresses are left unchanged. EEPROM addressing is not supported in GDB (for AVR targets).
+         *
+         * memoryOffsetsByType is a mapping of memory types to these known constants (which we're calling offsets).
+         * Because these offsets vary by target, the mapping lives here, in the GDB target descriptor.
+         */
         BiMap<Targets::TargetMemoryType, std::uint32_t> memoryOffsetsByType;
+
+        /**
+         * Sorted set of the known memory offsets (see memoryOffsetsByType).
+         */
         std::set<std::uint32_t> memoryOffsets;
     };
 }