ps2homebrew / hdl-dump

Install games in HDLoader format to APA-formatted hard drive
126 stars 25 forks source link

[ISSUE]: installing .bin/.cue CDs directly to drive fails copy data #83

Open endrift opened 2 days ago

endrift commented 2 days ago

Checks

Describe the issue

When attempting to use install_cd or install_dvd directly onto a drive (in my case, a /dev node on FreeBSD) when the source file is a .bin/.cue pair, while a plain UDF .iso file works fine. It exits with an opaque error: 00000002 (2): No such file or directory.

I attempted to debug this and discovered it's an issue with the data copying routine in iin_copy_ex--after copying several sectors, it attempts to read 512 2048-byte sectors, with an input_start_sector of 7134 out of the .bin file, which gets handled by img_base_read. output_start_sector is 242782072 but I'm not sure that's relevant. This redirects the read to al_read with 512 * the sector size specified in .cue file, which is 2352 bytes, or 1204224 bytes. However, the aligned_t has a smaller buffer_size, of 1048576.

I don't have the exact parameters anymore as my lldb session has scrolled out of my backlog by now, but I remember it fell through in al_read into the memmove branch, though I think the operation in question wound up being trivial. I believe usable_data_len was the same as buffer_size, so it ends up reading 0 bytes into the end of the buffer, as it's already full. This causes the read of 1204224 bytes to only return 1048576 bytes.

This short read from al_read gets passed directly out of img_base_read without checking if there was more that could be read. This results in iin_copy_ex getting confused that the read ended early at only ~445 sectors, and bailing out with this error.

I toyed with changing the number of sectors to read, but that just made problems worse. I also tried toying with the cache_size parameter for al_alloc, which did fix the issue, but I am unsure what the actual calculation should be.

This patch should work to change the rounding on the calculation to a ceil-equivalent, always rounding up to an extra integer instead of ever rounding down, but I am not familiar enough with the code to know if this is the right fix or even the right analysis:

diff --git a/iin_img_base.c b/iin_img_base.c
index 1aa7d94..b9db359 100644
--- a/iin_img_base.c
+++ b/iin_img_base.c
@@ -179,7 +179,7 @@ img_base_read(iin_t *iin,
             else {                        /* open the new input */
                 osal_handle_t in;
                 u_int32_t cache_size =
-                    ((img_base->raw_sector_size * IIN_NUM_SECTORS + img_base->raw_sector_size - 1) /
+                    ((img_base->raw_sector_size * IIN_NUM_SECTORS + part->device_sector_size - 1) /
                      part->device_sector_size);
                 result = osal_open(part->input_path, &in, 1); /* with no cache */
                 if (result == OSAL_OK) {

Console model

N/A

endrift commented 2 days ago

Forgot to mention: I hit this on both 32c296c69cf9c263fcbe035004aa28c345b3b279 and the v47 tag.