From ee0600cdbf5166ad7ef96c7ce281be05bc09342c Mon Sep 17 00:00:00 2001
From: water111 <48171810+water111@users.noreply.github.com>
Date: Sat, 18 Jan 2025 16:07:14 -0500
Subject: [PATCH] [jak3] Improve overlord file size handling (#3828)

Try to avoid crashes/corruption if a file changes size after you've
started the game.

Co-authored-by: water111 <awaterford1111445@gmail.com>
---
 game/overlord/jak3/dvd_driver.cpp | 12 +++++++++---
 game/overlord/jak3/dvd_driver.h   |  1 +
 game/overlord/jak3/iso_cd.cpp     | 18 ++++++++++--------
 game/overlord/jak3/isocommon.h    |  3 ++-
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/game/overlord/jak3/dvd_driver.cpp b/game/overlord/jak3/dvd_driver.cpp
index 4cc1cc7f1b..917b811543 100644
--- a/game/overlord/jak3/dvd_driver.cpp
+++ b/game/overlord/jak3/dvd_driver.cpp
@@ -401,6 +401,7 @@ void CDvdDriver::read_from_file(const jak3::Block* block) {
               selected_entry->def ? selected_entry->def->name.data : "NONE", fd->name.data);
     if (selected_entry->def) {
       fclose(selected_entry->fp);
+      selected_entry->size = 0;
     }
 
     selected_entry->def = fd;
@@ -408,6 +409,11 @@ void CDvdDriver::read_from_file(const jak3::Block* block) {
     if (!selected_entry->fp) {
       lg::die("Failed to open {} {}", fd->full_path, strerror(errno));
     }
+    // get the size of the file we actually opened, rather than caching the size at startup to
+    // support changing the file length after the game has started.
+    fseek(selected_entry->fp, 0, SEEK_END);
+    selected_entry->size = ftell(selected_entry->fp);
+    fseek(selected_entry->fp, 0, SEEK_SET);
     selected_entry->offset_in_file = 0;
   }
 
@@ -417,7 +423,7 @@ void CDvdDriver::read_from_file(const jak3::Block* block) {
   const u64 desired_offset = block->params.sector_num * 0x800;
 
   // see if we're reading entirely past the end of the file
-  if (desired_offset >= fd->length) {
+  if (desired_offset >= selected_entry->size) {
     return;
   }
 
@@ -432,14 +438,14 @@ void CDvdDriver::read_from_file(const jak3::Block* block) {
 
   // read
   s64 read_length = block->params.num_sectors * 0x800;
-  s64 extra_length = read_length + desired_offset - fd->length;
+  s64 extra_length = read_length + desired_offset - selected_entry->size;
   if (extra_length > 0) {
     read_length -= extra_length;
   }
   auto ret = fread(block->params.destination, read_length, 1, selected_entry->fp);
   if (ret != 1) {
     lg::die("Failed to read {} {}, size {} of {} (ret {})", fd->full_path, strerror(errno),
-            read_length, fd->length, ret);
+            read_length, selected_entry->size, ret);
   }
   selected_entry->offset_in_file += read_length;
 }
diff --git a/game/overlord/jak3/dvd_driver.h b/game/overlord/jak3/dvd_driver.h
index fc3f91c60f..663c9e7012 100644
--- a/game/overlord/jak3/dvd_driver.h
+++ b/game/overlord/jak3/dvd_driver.h
@@ -130,6 +130,7 @@ class CDvdDriver {
   struct FileCacheEntry {
     const ISOFileDef* def = nullptr;
     FILE* fp = nullptr;
+    size_t size = 0;
     u32 last_use_count = 0;
     u64 offset_in_file = 0;
   };
diff --git a/game/overlord/jak3/iso_cd.cpp b/game/overlord/jak3/iso_cd.cpp
index 4d731df8ab..d416ef9283 100644
--- a/game/overlord/jak3/iso_cd.cpp
+++ b/game/overlord/jak3/iso_cd.cpp
@@ -381,6 +381,7 @@ ISOFileDef* CISOCDFileSystem::FindIN(const jak3::ISOName* name) {
  * Get the length of a file, in bytes.
  */
 int CISOCDFileSystem::GetLength(const jak3::ISOFileDef* file) {
+  // actually open the file and get the length, in case it changed.
   // return file->length;
   lg::info("getlength");
   file_util::assert_file_exists(file->full_path.c_str(), "CISOCDFileSystem GetLength");
@@ -405,7 +406,10 @@ CBaseFile* CISOCDFileSystem::Open(const jak3::ISOFileDef* file_def,
   ASSERT(file_kind == 1);
 
   file->m_FileKind = CBaseFile::Kind::NORMAL;
-  file->m_nLength = file_def->length;
+  // get the length again. Note that the file length could still change in between here and
+  // when we actually go to read it... but this is unlikely and not easy to support - we'd need to
+  // move the FILE* into the CBaseFile.
+  file->m_nLength = GetLength(file_def);
   file->m_LengthPages = (0x7fff + file->m_nLength) >> 0xf;
   if (file->m_LengthPages < 1) {
     ASSERT_NOT_REACHED();
@@ -494,11 +498,6 @@ void CISOCDFileSystem::ReadDirectory() {
       MakeISOName(&e.name, file_name.c_str());
       e.full_path =
           fmt::format("{}/out/jak3/iso/{}", file_util::get_jak_project_dir().string(), file_name);
-      auto* fp = file_util::open_file(e.full_path, "rb");
-      ASSERT(fp);
-      fseek(fp, 0, SEEK_END);
-      e.length = ftell(fp);
-      fclose(fp);
     }
   }
 }
@@ -513,8 +512,11 @@ void CISOCDFileSystem::LoadMusicTweaks() {
   if (file) {
     auto fp = file_util::open_file(file->full_path, "rb");
     ASSERT(fp);
-    ASSERT(file->length <= sizeof(gMusicTweakInfo));
-    auto ret = fread(&gMusicTweakInfo, file->length, 1, fp);
+    fseek(fp, 0, SEEK_END);
+    auto size = ftell(fp);
+    fseek(fp, 0, SEEK_SET);
+    ASSERT(size <= sizeof(gMusicTweakInfo));
+    auto ret = fread(&gMusicTweakInfo, size, 1, fp);
     ASSERT(ret == 1);
     fclose(fp);
   } else {
diff --git a/game/overlord/jak3/isocommon.h b/game/overlord/jak3/isocommon.h
index a6f4a720ab..a19b8f4c9c 100644
--- a/game/overlord/jak3/isocommon.h
+++ b/game/overlord/jak3/isocommon.h
@@ -121,7 +121,8 @@ struct ISOName {
 struct ISOFileDef {
   ISOName name;
   std::string full_path;  // pc
-  u32 length;
+  // removed because PC doesn't assume constant file sizes.
+  // u32 length;
 };
 
 struct VagDirEntry {