Sanitize and improve the GB header parsing (#1109)

Further testing showed some limitations with the current GB header
parsing. Many homebrew and some licensed cartridges do not follow the
expected format. In particular, the manufacturer code used in the "new
format" is not always used properly.

Some mapper flags seem to not actually exist in practice. These have
been documented as comments.

The header parsing code was creating string objects with '\0' bytes,
this is now properly sanitized.

The destination code flag was not being set properly. This has been
fixed.

src/gb/gbCartData.cpp

@@ -19,14 +19,33 @@ struct gbRomHeader {
     uint8_t entry_point[4];
     uint8_t nintendo_logo[0x30];
     union {
-        uint8_t old_title[16];
+        // The original format.
+        struct {
+            uint8_t title[18];
+        } dmg;
+        // Some homebrew use this format. We also use this to workaround the
+        // manufacturer code being incorrectly set in some commercial carts.
+        struct {
+            uint8_t title[15];
+            uint8_t cgb_flag;
+            uint8_t padding[2];
+        } hb;
+        // The SGB format. The title can be up to 16 bytes long, since the last
+        // byte was not (yet) used for CGB identification.
+        struct {
+            uint8_t title[16];
+            uint8_t new_licensee_code[2];
+        } sgb;
+        // The final-ish CGB format. Some homebrew and commecial carts do not
+        // actually set the manufacturer code field properly so we revert to
+        // the "homebrew format" for these.
         struct {
             uint8_t title[11];
             uint8_t manufacturer_code[4];
             uint8_t cgb_flag;
-        } new_format;
+            uint8_t new_licensee_code[2];
+        } cgb;
     };
-    uint8_t new_licensee_code[2];
     uint8_t sgb_flag;
     uint8_t cartridge_type;
     uint8_t rom_size;
@@ -58,6 +77,26 @@ std::string old_licensee_to_string(uint8_t licensee) {
     return std::string(result.begin(), result.end());
 }
 
+template <typename T, std::size_t N>
+constexpr std::size_t array_size(const T (&)[N]) {
+    return N;
+}
+
+// Constructs a "real" string from a raw buffer. The buffer could have a null
+// value. If it does, the string will be truncated at that point.
+template <typename T>
+std::string from_buffer(const T& buffer) {
+    // The string is null terminated, so we can use std::find to find the end.
+    return std::string(buffer,
+                       std::find(buffer, buffer + array_size(buffer), '\0'));
+}
+
+bool is_valid_manufacturer_code(const std::string& manufacturer_code) {
+    return manufacturer_code.size() == 4 &&
+           std::all_of(manufacturer_code.begin(), manufacturer_code.end(),
+                       [](char c) { return std::isalnum(c); });
+}
+
 constexpr size_t kHeaderGlobalChecksumAdress = 0x14e;
 
 uint16_t get_rom_checksum(const uint8_t* romData, size_t romDataSize) {
@@ -92,8 +131,8 @@ bool is_game_shark(const gbRomHeader* romHeader) {
         0x41, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x20,
         0x52, 0x65, 0x70, 0x6c, 0x61, 0x79, 0x20};
 
-    return std::equal(romHeader->old_title,
+    return std::equal(romHeader->dmg.title,
-                      romHeader->old_title + kGameSharkTitle.size(),
+                      romHeader->dmg.title + kGameSharkTitle.size(),
                       kGameSharkTitle.begin(), kGameSharkTitle.end());
 }
 
@@ -162,47 +201,58 @@ gbCartData::gbCartData(const uint8_t* romData, size_t romDataSize) {
         return;
     }
 
+    // SGB support flag.
+    sgb_flag_ = header->sgb_flag;
+    sgb_support_ = (sgb_flag_ == kSgbSetFlag);
+
+    // CGB support flag. This needs to be parsed now to work around some
+    // incorrect homebrew ROMs.
+    cgb_flag_ = header->cgb.cgb_flag;
+    if (cgb_flag_ == kCgbSupportedFlag) {
+        cgb_support_ = CGBSupport::kSupported;
+    } else if (cgb_flag_ == kCgbRequiredFlag) {
+        cgb_support_ = CGBSupport::kRequired;
+    } else {
+        // Any other value must be interpreted as being part of the title,
+        // with the exception of some homebrew.
+        cgb_support_ = CGBSupport::kNone;
+    }
+
     old_licensee_code_ = header->old_licensee_code;
     if (old_licensee_code_ == kNewManufacturerCode) {
         // New licensee code format.
         header_type_ = RomHeaderType::kNewLicenseeCode;
-        title_ =
+        maker_code_ = from_buffer(header->cgb.new_licensee_code);
-            std::string(reinterpret_cast<const char*>(header->new_format.title),
-                        sizeof(header->new_format.title));
-        maker_code_ = std::string(
-            reinterpret_cast<const char*>(header->new_licensee_code),
-            sizeof(header->new_licensee_code));
-        manufacturer_code_ = std::string(
-            reinterpret_cast<const char*>(header->new_format.manufacturer_code),
-            sizeof(header->new_format.manufacturer_code));
 
-        // CGB support flag.
+        if (cgb_support_ == CGBSupport::kNone) {
-        cgb_flag_ = header->new_format.cgb_flag;
+            title_ = from_buffer(header->sgb.title);
-        if (cgb_flag_ == kCgbSupportedFlag) {
-            cgb_support_ = CGBSupport::kSupported;
-        } else if (cgb_flag_ == kCgbRequiredFlag) {
-            cgb_support_ = CGBSupport::kRequired;
         } else {
-            cgb_support_ = CGBSupport::kNone;
+            manufacturer_code_ = from_buffer(header->cgb.manufacturer_code);
+            if (is_valid_manufacturer_code(manufacturer_code_)) {
+                title_ = from_buffer(header->cgb.title);
+            } else {
+                // Some ROMS and homebrew do not actually set this field.
+                // Interpret the title as a homebrew title instead.
+                manufacturer_code_.clear();
+                title_ = from_buffer(header->hb.title);
+            }
         }
     } else {
         // Old licensee code format.
         header_type_ = RomHeaderType::kOldLicenseeCode;
 
-        title_ = std::string(reinterpret_cast<const char*>(header->old_title),
+        if (cgb_support_ == CGBSupport::kNone) {
-                             sizeof(header->old_title));
+            // Rese the CGB flag here. This is part of the title.
+            cgb_flag_ = 0;
+            title_ = from_buffer(header->dmg.title);
+        } else {
+            // This is (incorrectly) used by some homebrew.
+            title_ = from_buffer(header->hb.title);
+        }
 
         maker_code_ = old_licensee_to_string(header->old_licensee_code);
-
-        cgb_flag_ = 0;
-        cgb_support_ = CGBSupport::kNone;
-        manufacturer_code_ = "";
     }
 
-    // SGB support flag.
-    sgb_flag_ = header->sgb_flag;
-    sgb_support_ = header->sgb_flag == kSgbSetFlag;
-
     // We may have to skip the RAM size calculation for some mappers.
     bool skip_ram = false;
     mapper_flag_ = header->cartridge_type;
@@ -222,11 +272,13 @@ gbCartData::gbCartData(const uint8_t* romData, size_t romDataSize) {
             break;
         case 0x05:
             // MBC2 header does not specify a RAM size so set it here.
+            // This specific flag does not seem to exist in any cart.
             mapper_type_ = MapperType::kMbc2;
             ram_size_ = k512B;
             skip_ram = true;
             break;
         case 0x06:
+            // MBC2 header does not specify a RAM size so set it here.
             mapper_type_ = MapperType::kMbc2;
             ram_size_ = k512B;
             skip_ram = true;
@@ -240,16 +292,20 @@ gbCartData::gbCartData(const uint8_t* romData, size_t romDataSize) {
             has_battery_ = true;
             break;
         case 0x0b:
+            // No cart seems to have a proper dump for this.
             mapper_type_ = MapperType::kMmm01;
             break;
         case 0x0c:
+            // No cart seems to have a proper dump for this.
             mapper_type_ = MapperType::kMmm01;
             break;
         case 0x0d:
+            // No cart seems to have a proper dump for this.
             mapper_type_ = MapperType::kMmm01;
             has_battery_ = true;
             break;
         case 0x0f:
+            // This specific flag does not seem to exist in any cart.
             mapper_type_ = MapperType::kMbc3;
             has_rtc_ = true;
             has_battery_ = true;
@@ -260,9 +316,11 @@ gbCartData::gbCartData(const uint8_t* romData, size_t romDataSize) {
             has_battery_ = true;
             break;
         case 0x11:
+            // This specific flag does not seem to exist in any cart.
             mapper_type_ = MapperType::kMbc3;
             break;
         case 0x12:
+            // This specific flag does not seem to exist in any cart.
             mapper_type_ = MapperType::kMbc3;
             break;
         case 0x13:
@@ -284,6 +342,7 @@ gbCartData::gbCartData(const uint8_t* romData, size_t romDataSize) {
             has_rumble_ = true;
             break;
         case 0x1d:
+            // This specific flag does not seem to exist in any cart.
             mapper_type_ = MapperType::kMbc5;
             has_rumble_ = true;
             break;
@@ -409,6 +468,13 @@ gbCartData::gbCartData(const uint8_t* romData, size_t romDataSize) {
 
     version_flag_ = header->version_number;
 
+    destination_code_flag_ = header->destination_code;
+    if (destination_code_flag_ == 0x00) {
+        destination_code_ = DestinationCode::kJapanese;
+    } else if (destination_code_flag_ == 0x01) {
+        destination_code_ = DestinationCode::kWorldwide;
+    }
+
     // Calculate the header checksum.
     header_checksum_ = header->header_checksum;
     uint8_t checksum = 0;