armsx/ARMSX3
0
0
Fork 0
mirror of https://github.com/ARMSX2/ARMSX3.git synced 2026-08-24 16:58:52 -07:00
Code Issues Packages Projects Releases Wiki Activity

patch_engine: stop save_patches from destroying the file it rewrites

Browse Source 
Two defects on the same write path, both reachable today from Download
database and from a local patch import.

1. The file was opened with fs::rewrite (write + create + trunc), streamed
   into, and the write result discarded -- save_patches returned true
   unconditionally. A write that fails part way (out of space, process
   killed) therefore leaves a truncated patch.yml behind, and load() rejects
   the whole file on a parse error, so the failure costs the user every patch
   they had. There is no way to rebuild it from inside the app either:
   import_patches refuses to write when load() fails, so both import paths
   return -1 from then on.

   save_config, 70 lines up in the same file, already writes through
   fs::pending_file and checks the result. save_patches now does the same.

2. The address element was always emitted as fmt::format("0x%.8x", offset).
   For move_file and hide_file that element is a VFS path, not a number:
   load() keeps the text in original_offset and skips the u32 validation for
   those two types. So a round trip turned a path into 0x00000000, and the
   loader accepted it back -- the patch still lists and still toggles, it just
   silently stops matching anything. Re-downloading does not repair it,
   because append_patches discards an incoming patch whose Patch Version is
   not strictly greater than the stored one.

   The emit is now gated on patch_type_uses_hex_offset, the predicate that
   already existed for this and was used only on the load side.

   The numeric branch deliberately keeps using offset rather than
   original_offset: an address modifier is folded into offset at load time,
   and the flat form emitted here has nowhere to put it.

Both predate the Android patch work and apply to upstream RPCS3 unchanged;
they are in this branch because the bundled-patch import adds another caller
of save_patches.

Verified on device (arm64, Android 15). A patch.yml seeded with move_file and
hide_file entries was put through an import that merges a new patch, which is
what forces the rewrite. After it:

  - [move_file, /dev_bdvd/PS3_GAME/USRDIR/probe.bik, /dev_bdvd/PS3_GAME/USRDIR/probe.bik.bak]
  - [hide_file, /dev_bdvd/PS3_GAME/USRDIR/hidden.bik, ""]

Both paths survived; before this change they would read 0x00000000. All three
top-level hashes in the file (the two seeded, plus the merged one) were still
present and parseable afterwards.

Not verified: the failure path in (1). Forcing a short write mid-rewrite
(ENOSPC or a kill inside save_patches) was not exercised, so the atomicity is
argued from fs::pending_file's contract and from parity with save_config, not
from a reproduced failure.
This commit is contained in:
Zulux91
2026-08-17 03:20:43 -05:00
parent 4179f23e20
commit 20c854aeb3
1 changed files with 29 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Utilities/bin_patch.cpp
+29 -9
View File
@@ -1803,13 +1803,6 @@ static void append_patches(patch_engine::patch_map& existing_patches, const patc
bool patch_engine::save_patches(const patch_map& patches, const std::string& path, std::stringstream* log_messages)
{
fs::file file(path, fs::rewrite);
if (!file)
{
append_log_message(log_messages, fmt::format("Failed to open patch file %s (%s)", path, fs::g_tls_error), &patch_log.fatal);
return false;
}
YAML::Emitter out;
out << YAML::BeginMap;
out << patch_key::version << patch_engine_version;
@@ -1904,7 +1897,24 @@ bool patch_engine::save_patches(const patch_map& patches, const std::string& pat
out << YAML::Flow;
out << YAML::BeginSeq;
out << fmt::format("%s", data.type);
out << fmt::format("0x%.8x", data.offset);
// move_file and hide_file carry a VFS path in the address element instead of a
// number. load() keeps that text in original_offset and skips the u32 validation for
// them, so formatting it numerically here would write out 0x00000000 and the loader
// would accept it back as a patch that silently never matches anything.
//
// The numeric branch deliberately uses offset rather than original_offset: an
// address modifier is folded into offset at load time, and the flat form emitted
// here has nowhere to put it.
if (patch_type_uses_hex_offset(data.type))
{
out << fmt::format("0x%.8x", data.offset);
}
else
{
out << data.original_offset;
}
out << data.original_value;
out << YAML::EndSeq;
}
@@ -1918,7 +1928,17 @@ bool patch_engine::save_patches(const patch_map& patches, const std::string& pat
out << YAML::EndMap;
file.write(out.c_str(), out.size());
// Write through a temporary and rename on success, as save_config already does. A truncating
// in-place write that fails part way (out of space, process killed) leaves a half-written file,
// and load() rejects the whole file on a parse error -- so a failure here costs the user every
// patch they had, with no way to rebuild it from inside the app.
fs::pending_file file(path);
if (!file.file || file.file.write(out.c_str(), out.size()) < out.size() || !file.commit())
{
append_log_message(log_messages, fmt::format("Failed to write patch file %s (%s)", path, fs::g_tls_error), &patch_log.fatal);
return false;
}
return true;
}