Skip to content

fix(UnZipFile): Added fallback to previous method when dealing with b… #1308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BotellaA merged 1 commit into from
Jul 22, 2026
Merged

fix(UnZipFile): Added fallback to previous method when dealing with b… #1308

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions src/geode/basic/zip_file.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include <fstream>
#include <string_view>

#include <mz.h>

Check failure on line 30 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:30:10 [clang-diagnostic-error] 

'mz.h' file not found
#include <mz_strm.h>
#include <mz_strm_mem.h>
#include <mz_zip.h>
Expand All @@ -39,7 +39,7 @@
namespace
{
std::filesystem::path create_directory(
std::string_view file, std::string_view temp_filename )

Check warning on line 42 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:42:9 [bugprone-easily-swappable-parameters] 

2 adjacent parameters of 'create_directory' of similar type ('std::string_view') are easily swapped by mistake
{
const auto file_string = geode::to_string( file );
auto directory = std::filesystem::path{ file_string }.parent_path()
Expand All @@ -51,13 +51,13 @@

namespace geode
{
class ZipFile::Impl

Check warning on line 54 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:54:20 [cppcoreguidelines-special-member-functions] 

class 'Impl' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator
{
public:
Impl( std::string_view file, std::string_view archive_temp_filename )
{
directory_ = create_directory( file, archive_temp_filename );
writer_ = mz_zip_writer_create();

Check warning on line 60 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:60:13 [cppcoreguidelines-prefer-member-initializer] 

'writer_' should be initialized in a member initializer of the constructor
mz_zip_writer_set_compress_method(
writer_, MZ_COMPRESS_METHOD_STORE );
const auto status = mz_zip_writer_open_file(
Expand All @@ -72,7 +72,7 @@

~Impl()
{
if( writer_ )

Check warning on line 75 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:75:17 [readability-implicit-bool-conversion] 

implicit conversion 'void *' -> 'bool'
{
mz_zip_writer_close( writer_ );
mz_zip_writer_delete( &writer_ );
Expand Down Expand Up @@ -103,7 +103,7 @@
std::filesystem::remove( file_path );
}

std::string directory() const

Check warning on line 106 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:106:9 [modernize-use-nodiscard] 

function 'directory' should be marked [[nodiscard]]
{
return directory_.string();
}
Expand Down Expand Up @@ -137,35 +137,37 @@
return impl_->directory();
}

class UnzipFile::Impl

Check warning on line 140 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:140:22 [cppcoreguidelines-special-member-functions] 

class 'Impl' defines a non-default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator
{
public:
Impl( std::string_view file, std::string_view unarchive_temp_filename )

Check warning on line 143 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:143:9 [cppcoreguidelines-pro-type-member-init] 

constructor does not initialize these fields: zip_data_
{
directory_ = create_directory( file, unarchive_temp_filename );
if( !load_zip_into_memory( file ) )
if( !load_zip_into_memory( file ) || !open_reader() )
{
throw OpenGeodeBasicException( nullptr,
OpenGeodeException::TYPE::internal,
"[UnzipFile] Failed to read zip file into memory" );
}
if( !open_reader() )
{
throw OpenGeodeBasicException( nullptr,
OpenGeodeException::TYPE::internal,
"[UnzipFile] Error opening zip for reading" );
Logger::info( "[UnzipFile] Couldn't open zip in memory, trying "
"to open on disk, this could take more time" );
reader_ = nullptr;
memory_stream_ = nullptr;
if( !create_reader_from_disk( file ) )
{
std::filesystem::remove_all( directory_ );
throw OpenGeodeBasicException( nullptr,
OpenGeodeException::TYPE::internal,
"[UnzipFile] Error opening zip for reading" );
}
}
}

~Impl()
{
if( reader_ )

Check warning on line 164 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:164:17 [readability-implicit-bool-conversion] 

implicit conversion 'void *' -> 'bool'
{
mz_zip_reader_close( reader_ );
mz_zip_reader_delete( &reader_ );
reader_ = nullptr;
}
if( memory_stream_ )

Check warning on line 170 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:170:17 [readability-implicit-bool-conversion] 

implicit conversion 'void *' -> 'bool'
{
mz_stream_close( memory_stream_ );
mz_stream_delete( &memory_stream_ );
Expand All @@ -176,7 +178,7 @@

void extract_all() const
{
constexpr size_t BUF_SIZE = 1024 * 1024; // 1 MB

Check warning on line 181 in src/geode/basic/zip_file.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/basic/zip_file.cpp:181:41 [bugprone-implicit-widening-of-multiplication-result] 

performing an implicit widening conversion to type 'const size_t' (aka 'const unsigned long') of a multiplication performed in type 'int'
std::vector< uint8_t > buffer( BUF_SIZE );
int status = mz_zip_reader_goto_first_entry( reader_ );
while( status == MZ_OK )
Expand All @@ -190,8 +192,8 @@
}
auto out_path = directory_ / info->filename;
std::filesystem::create_directories( out_path.parent_path() );
FILE* f = fopen( out_path.string().c_str(), "wb" );
if( !f )
FILE* file = fopen( out_path.string().c_str(), "wb" );
if( !file )
{
status = mz_zip_reader_goto_next_entry( reader_ );
continue;
Expand All @@ -204,11 +206,11 @@
reader_, buffer.data(), BUF_SIZE ) )
> 0 )
{
fwrite( buffer.data(), 1, bytes_read, f );
fwrite( buffer.data(), 1, bytes_read, file );
}
mz_zip_reader_entry_close( reader_ );
}
fclose( f );
fclose( file );
status = mz_zip_reader_goto_next_entry( reader_ );
}
}
Expand Down Expand Up @@ -244,6 +246,13 @@
return mz_zip_reader_open( reader_, memory_stream_ ) == MZ_OK;
}

bool create_reader_from_disk( std::string_view file )
{
reader_ = mz_zip_reader_create();
return mz_zip_reader_open_file( reader_, to_string( file ).c_str() )
== MZ_OK;
}

private:
std::filesystem::path directory_;
std::vector< uint8_t > zip_data_;
Expand Down
Loading