From cd72c04a13e25506b768e71a6ac6a4ac26c231df Mon Sep 17 00:00:00 2001 From: Jamiras <32680403+Jamiras@users.noreply.github.com> Date: Sat, 13 Jan 2024 08:56:26 -0700 Subject: [PATCH] fix read across region boundary (#308) --- src/rc_libretro.c | 76 +++++++++++++++++++-------------------- test/test_rc_libretro.c | 79 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 39 deletions(-) diff --git a/src/rc_libretro.c b/src/rc_libretro.c index 70bf06be..93c5cd50 100644 --- a/src/rc_libretro.c +++ b/src/rc_libretro.c @@ -324,27 +324,38 @@ uint8_t* rc_libretro_memory_find(const rc_libretro_memory_regions_t* regions, ui uint32_t rc_libretro_memory_read(const rc_libretro_memory_regions_t* regions, uint32_t address, uint8_t* buffer, uint32_t num_bytes) { - uint32_t i; + uint32_t bytes_read = 0; uint32_t avail; + uint32_t i; for (i = 0; i < regions->count; ++i) { const size_t size = regions->size[i]; - if (address < size) { - if (regions->data[i] == NULL) - break; + if (address >= size) { + /* address is not in this block, adjust and look at next block */ + address -= (unsigned)size; + continue; + } - avail = (unsigned)(size - address); - if (avail < num_bytes) - return avail; + if (regions->data[i] == NULL) /* no memory associated to this block. abort */ + break; + avail = (unsigned)(size - address); + if (avail >= num_bytes) { + /* requested memory is fully within this block, copy and return it */ memcpy(buffer, ®ions->data[i][address], num_bytes); - return num_bytes; + bytes_read += num_bytes; + return bytes_read; } - address -= (unsigned)size; + /* copy whatever is available in this block, and adjust for the next block */ + memcpy(buffer, ®ions->data[i][address], avail); + buffer += avail; + bytes_read += avail; + num_bytes -= avail; + address = 0; } - return 0; + return bytes_read; } void rc_libretro_init_verbose_message_callback(rc_libretro_message_callback callback) { @@ -671,8 +682,7 @@ void rc_libretro_hash_set_init(struct rc_libretro_hash_set_t* hash_set, return; file_handle = rc_file_open(m3u_path); - if (!file_handle) - { + if (!file_handle) { rc_hash_error("Could not open playlist"); return; } @@ -682,8 +692,7 @@ void rc_libretro_hash_set_init(struct rc_libretro_hash_set_t* hash_set, rc_file_seek(file_handle, 0, SEEK_SET); m3u_contents = (char*)malloc((size_t)file_len + 1); - if (m3u_contents) - { + if (m3u_contents) { rc_file_read(file_handle, m3u_contents, (int)file_len); m3u_contents[file_len] = '\0'; @@ -696,23 +705,19 @@ void rc_libretro_hash_set_init(struct rc_libretro_hash_set_t* hash_set, while (isspace((int)*ptr)) ++ptr; - if (*ptr == '#') - { + if (*ptr == '#') { /* ignore comment unless it's the special SAVEDISK extension */ - if (memcmp(ptr, "#SAVEDISK:", 10) == 0) - { + if (memcmp(ptr, "#SAVEDISK:", 10) == 0) { /* get the path to the save disk from the frontend, assign it a bogus hash so * it doesn't get hashed later */ - if (get_image_path(index, image_path, sizeof(image_path))) - { + if (get_image_path(index, image_path, sizeof(image_path))) { const char save_disk_hash[33] = "[SAVE DISK]"; rc_libretro_hash_set_add(hash_set, image_path, -1, save_disk_hash); ++index; } } } - else - { + else { /* non-empty line, tally a file */ ++index; } @@ -726,8 +731,7 @@ void rc_libretro_hash_set_init(struct rc_libretro_hash_set_t* hash_set, free(m3u_contents); } - if (hash_set->entries_count > 0) - { + if (hash_set->entries_count > 0) { /* at least one save disk was found. make sure the core supports the #SAVEDISK: extension by * asking for the last expected disk. if it's not found, assume no #SAVEDISK: support */ if (!get_image_path(index - 1, image_path, sizeof(image_path))) @@ -759,13 +763,10 @@ void rc_libretro_hash_set_add(struct rc_libretro_hash_set_t* hash_set, struct rc_libretro_hash_entry_t* scan; struct rc_libretro_hash_entry_t* stop = hash_set->entries + hash_set->entries_count;; - if (path_djb2) - { + if (path_djb2) { /* attempt to match the path */ - for (scan = hash_set->entries; scan < stop; ++scan) - { - if (scan->path_djb2 == path_djb2) - { + for (scan = hash_set->entries; scan < stop; ++scan) { + if (scan->path_djb2 == path_djb2) { entry = scan; break; } @@ -775,19 +776,20 @@ void rc_libretro_hash_set_add(struct rc_libretro_hash_set_t* hash_set, if (!entry) { /* entry not found, allocate a new one */ - if (hash_set->entries_size == 0) - { + if (hash_set->entries_size == 0) { hash_set->entries_size = 4; hash_set->entries = (struct rc_libretro_hash_entry_t*) malloc(hash_set->entries_size * sizeof(struct rc_libretro_hash_entry_t)); } - else if (hash_set->entries_count == hash_set->entries_size) - { + else if (hash_set->entries_count == hash_set->entries_size) { hash_set->entries_size += 4; hash_set->entries = (struct rc_libretro_hash_entry_t*)realloc(hash_set->entries, hash_set->entries_size * sizeof(struct rc_libretro_hash_entry_t)); } + if (hash_set->entries == NULL) /* unexpected, but better than crashing */ + return; + entry = hash_set->entries + hash_set->entries_count++; } @@ -802,8 +804,7 @@ const char* rc_libretro_hash_set_get_hash(const struct rc_libretro_hash_set_t* h const uint32_t path_djb2 = rc_libretro_djb2(path); struct rc_libretro_hash_entry_t* scan = hash_set->entries; struct rc_libretro_hash_entry_t* stop = scan + hash_set->entries_count; - for (; scan < stop; ++scan) - { + for (; scan < stop; ++scan) { if (scan->path_djb2 == path_djb2) return scan->hash; } @@ -815,8 +816,7 @@ int rc_libretro_hash_set_get_game_id(const struct rc_libretro_hash_set_t* hash_s { struct rc_libretro_hash_entry_t* scan = hash_set->entries; struct rc_libretro_hash_entry_t* stop = scan + hash_set->entries_count; - for (; scan < stop; ++scan) - { + for (; scan < stop; ++scan) { if (memcmp(scan->hash, hash, sizeof(scan->hash)) == 0) return scan->game_id; } diff --git a/test/test_rc_libretro.c b/test/test_rc_libretro.c index cbcba767..e23ba815 100644 --- a/test/test_rc_libretro.c +++ b/test/test_rc_libretro.c @@ -85,7 +85,7 @@ static void test_memory_init_without_regions() { ASSERT_TRUE(memcmp(buffer3, &buffer1[2], 1) == 0); ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 7, buffer3, 4), 4); ASSERT_TRUE(memcmp(buffer3, &buffer1[7], 4) == 0); - ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, sizeof(buffer1) - 2, buffer3, 3), 2); + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, sizeof(buffer1) - 2, buffer3, 3), 3); /* read across boundary */ ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, sizeof(buffer1) + sizeof(buffer2) + 2, buffer3, 1), 0); } @@ -417,6 +417,80 @@ static void test_memory_init_from_memory_map_disconnect_gaps() { ASSERT_PTR_NULL(rc_libretro_memory_find(®ions, 0x0142)); } + +static void test_memory_read() +{ + rc_libretro_memory_regions_t regions; + /* intentionally put buffer3 between buffer1 and buffer2 for the read that spans buffers */ + uint8_t buffer1[8], buffer3[4], buffer2[8]; + const struct retro_memory_descriptor mmap_desc[] = { + { RETRO_MEMDESC_SYSTEM_RAM, &buffer1[0], 0, 0x000000U, 0, 0, 0x000008, "RAM A" }, + { RETRO_MEMDESC_SYSTEM_RAM, &buffer2[0], 0, 0x000008U, 0, 0, 0x000008, "RAM B" } + }; + const struct retro_memory_map mmap = { mmap_desc, sizeof(mmap_desc) / sizeof(mmap_desc[0]) }; + int i; + + for (i = 0; i < 8; ++i) { + buffer1[i] = i; + buffer2[i] = i + 8; + } + + ASSERT_TRUE(rc_libretro_memory_init(®ions, &mmap, libretro_get_core_memory_info, RC_CONSOLE_NINTENDO)); + + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 0, buffer3, 4), 4); + ASSERT_NUM_EQUALS(buffer3[0], 0); + ASSERT_NUM_EQUALS(buffer3[1], 1); + ASSERT_NUM_EQUALS(buffer3[2], 2); + ASSERT_NUM_EQUALS(buffer3[3], 3); + + /* only requesting two bytes, other two remain unmodified */ + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 3, buffer3, 2), 2); + ASSERT_NUM_EQUALS(buffer3[0], 3); + ASSERT_NUM_EQUALS(buffer3[1], 4); + ASSERT_NUM_EQUALS(buffer3[2], 2); + ASSERT_NUM_EQUALS(buffer3[3], 3); + + /* one two bytes are available in buffer1, the rest have to be read from buffer2 */ + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 6, buffer3, 4), 4); + ASSERT_NUM_EQUALS(buffer3[0], 6); + ASSERT_NUM_EQUALS(buffer3[1], 7); + ASSERT_NUM_EQUALS(buffer3[2], 8); /* this comes from buffer2 */ + ASSERT_NUM_EQUALS(buffer3[3], 9); + + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 8, buffer3, 4), 4); + ASSERT_NUM_EQUALS(buffer3[0], 8); + ASSERT_NUM_EQUALS(buffer3[1], 9); + ASSERT_NUM_EQUALS(buffer3[2], 10); + ASSERT_NUM_EQUALS(buffer3[3], 11); + + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 11, buffer3, 4), 4); + ASSERT_NUM_EQUALS(buffer3[0], 11); + ASSERT_NUM_EQUALS(buffer3[1], 12); + ASSERT_NUM_EQUALS(buffer3[2], 13); + ASSERT_NUM_EQUALS(buffer3[3], 14); + + /* only requesting 1 byte. other three remain unmodified */ + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 13, buffer3, 1), 1); + ASSERT_NUM_EQUALS(buffer3[0], 13); + ASSERT_NUM_EQUALS(buffer3[1], 12); + ASSERT_NUM_EQUALS(buffer3[2], 13); + ASSERT_NUM_EQUALS(buffer3[3], 14); + + /* only two bytes are available at address 14 */ + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 14, buffer3, 3), 2); + ASSERT_NUM_EQUALS(buffer3[0], 14); + ASSERT_NUM_EQUALS(buffer3[1], 15); + ASSERT_NUM_EQUALS(buffer3[2], 13); + ASSERT_NUM_EQUALS(buffer3[3], 14); + + /* no bytes are available at invalid address */ + ASSERT_NUM_EQUALS(rc_libretro_memory_read(®ions, 16, buffer3, 4), 0); + ASSERT_NUM_EQUALS(buffer3[0], 14); + ASSERT_NUM_EQUALS(buffer3[1], 15); + ASSERT_NUM_EQUALS(buffer3[2], 13); + ASSERT_NUM_EQUALS(buffer3[3], 14); +} + static void test_hash_set_add_single() { rc_libretro_hash_set_t hash_set; const char hash[] = "ABCDEF01234567899876543210ABCDEF"; @@ -755,6 +829,9 @@ void test_rc_libretro(void) { TEST(test_memory_init_from_memory_map_out_of_order); TEST(test_memory_init_from_memory_map_disconnect_gaps); + /* rc_libretro_memory_read */ + TEST(test_memory_read) + /* rc_libretro_hash_set_t */ TEST(test_hash_set_add_single); TEST(test_hash_set_update_single);