Skip to content

Conversation

@chaoticgd
Copy link
Member

@chaoticgd chaoticgd commented Dec 31, 2025

Description of Changes

This introduces new classes for reading/writing zip files to replace the existing functions in ZipHelpers.h. These provide a proper RAII wrapper around libzip that integrates with our Error class.

Rationale behind Changes

  • There are some libzip call sites where we aren't checking for errors. I already fixed one of these in a previous PR (SaveState: Add a setting to let users choose how errors should be reported when loading/saving states #13638) but there are others too.
  • The zip_open_managed and zip_open_buffer_managed functions are both affected by the following issues:
    • Because zip_close is called from a custom std::unique_ptr deleter, there's no way to pass error information up the call stack, and since zip_close is the step that causes the original file on disk to be replaced with the new one, this error information is meaningful (for example, if there is a permissions problem).
    • The code seems to think the return value of zip_close is an error code, which it is not.
  • There are some cases where incorrect flags are passed to libzip functions. For example, in ReadFileInZipToContainer, ZIP_FL_NOCASE is passed to functions that don't check for it.
  • As for why I opted to write my own wrapper classes instead of using an existing C++ wrapper library, this way the classes can be integrated with our own Error class, and I haven't found an existing one that I'm confident is high quality.

Suggested Testing Steps

Not yet ready for testing.

Did you use AI to help find, test, or implement this issue or feature?

No.

@chaoticgd chaoticgd added this to the Release 2.8 milestone Dec 31, 2025
@chaoticgd chaoticgd force-pushed the zip_rewrite branch 3 times, most recently from 29ad262 to e94b158 Compare December 31, 2025 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant