Skip to content

Commit

Permalink
[jak3] Improve overlord file size handling (#3828)
Browse files Browse the repository at this point in the history
Try to avoid crashes/corruption if a file changes size after you've
started the game.

Co-authored-by: water111 <awaterford1111445@gmail.com>
  • Loading branch information
water111 and water111 authored Jan 18, 2025
1 parent a29fca9 commit ee0600c
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 12 deletions.
12 changes: 9 additions & 3 deletions game/overlord/jak3/dvd_driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,19 @@ 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;
selected_entry->fp = file_util::open_file(fd->full_path, "rb");
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;
}

Expand All @@ -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;
}

Expand All @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions game/overlord/jak3/dvd_driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
18 changes: 10 additions & 8 deletions game/overlord/jak3/iso_cd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion game/overlord/jak3/isocommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ee0600c

Please sign in to comment.