Skip to content

Commit

Permalink
fix read across region boundary (#308)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jamiras
Jamiras authored Jan 13, 2024
1 parent ff20c82 commit cd72c04
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 39 deletions.
76 changes: 38 additions & 38 deletions src/rc_libretro.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, &regions->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, &regions->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) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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';

Expand All @@ -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;
}
Expand All @@ -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)))
Expand Down Expand Up @@ -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;
}
Expand All @@ -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++;
}

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
79 changes: 78 additions & 1 deletion test/test_rc_libretro.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(&regions, 7, buffer3, 4), 4);
ASSERT_TRUE(memcmp(buffer3, &buffer1[7], 4) == 0);
ASSERT_NUM_EQUALS(rc_libretro_memory_read(&regions, sizeof(buffer1) - 2, buffer3, 3), 2);
ASSERT_NUM_EQUALS(rc_libretro_memory_read(&regions, sizeof(buffer1) - 2, buffer3, 3), 3); /* read across boundary */
ASSERT_NUM_EQUALS(rc_libretro_memory_read(&regions, sizeof(buffer1) + sizeof(buffer2) + 2, buffer3, 1), 0);
}

Expand Down Expand Up @@ -417,6 +417,80 @@ static void test_memory_init_from_memory_map_disconnect_gaps() {
ASSERT_PTR_NULL(rc_libretro_memory_find(&regions, 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(&regions, &mmap, libretro_get_core_memory_info, RC_CONSOLE_NINTENDO));

ASSERT_NUM_EQUALS(rc_libretro_memory_read(&regions, 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(&regions, 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(&regions, 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(&regions, 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(&regions, 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(&regions, 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(&regions, 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(&regions, 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";
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit cd72c04

Please sign in to comment.