Skip to content

Commit

Permalink
Improve logging of errors when files fail to open
Browse files Browse the repository at this point in the history
  • Loading branch information
hexagonrecursion committed Jan 20, 2025
1 parent 685c563 commit 8994b0c
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 11 deletions.
2 changes: 2 additions & 0 deletions colobot-base/src/common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ target_sources(Colobot-Base PRIVATE
resources/outputstream.h
resources/outputstreambuffer.cpp
resources/outputstreambuffer.h
resources/physfs_utils.cpp
resources/physfs_utils.h
resources/resourcemanager.cpp
resources/resourcemanager.h
resources/sdl_file_wrapper.cpp
Expand Down
3 changes: 2 additions & 1 deletion colobot-base/src/common/resources/inputstreambuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "common/resources/inputstreambuffer.h"

#include "common/resources/resourcemanager.h"
#include "common/resources/physfs_utils.h"

#include <stdexcept>
#include <sstream>
Expand All @@ -46,7 +47,7 @@ CInputStreamBuffer::~CInputStreamBuffer()
void CInputStreamBuffer::open(const std::filesystem::path& path)
{
if (PHYSFS_isInit())
m_file = PHYSFS_openRead(CResourceManager::CleanPath(path.generic_u8string()).c_str());
m_file = LoudOpenRead(CResourceManager::CleanPath(path.generic_u8string()).c_str());
}


Expand Down
5 changes: 3 additions & 2 deletions colobot-base/src/common/resources/outputstreambuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "common/resources/outputstreambuffer.h"

#include "common/resources/resourcemanager.h"
#include "common/resources/physfs_utils.h"

#include <stdexcept>
#include <sstream>
Expand Down Expand Up @@ -48,9 +49,9 @@ void COutputStreamBuffer::open(const std::filesystem::path& path, std::ios_base:
if (PHYSFS_isInit())
{
if ( mode == std::ios_base::out )
m_file = PHYSFS_openWrite(CResourceManager::CleanPath(path.generic_u8string()).c_str());
m_file = LoudOpenWrite(CResourceManager::CleanPath(path.generic_u8string()).c_str());
else if ( mode == std::ios_base::app )
m_file = PHYSFS_openAppend(CResourceManager::CleanPath(path.generic_u8string()).c_str());
m_file = LoudOpenAppend(CResourceManager::CleanPath(path.generic_u8string()).c_str());
}
}

Expand Down
98 changes: 98 additions & 0 deletions colobot-base/src/common/resources/physfs_utils.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#include "common/resources/physfs_utils.h"

#include "common/logger.h"

#include <string>
#include <locale>
#include <sstream>
#include <ios>
#include <iomanip>

namespace {
// https://codereview.stackexchange.com/q/295100/238668
std::string encode(const char *s)
{
if(s == nullptr) return "nullptr";
static const std::locale cLocale("C");
std::stringstream out;
out << '"';
for(; *s; ++s)
{
switch(*s)
{
case '"':
case '?': // May need escaping due to trigraphs
case '\\':
out << '\\' << *s; break;
case '\a': out << "\\a"; break;
case '\b': out << "\\b"; break;
case '\f': out << "\\f"; break;
case '\n': out << "\\n"; break;
case '\r': out << "\\r"; break;
case '\t': out << "\\t"; break;
case '\v': out << "\\v"; break;
default:
if(std::isprint(*s, cLocale))
{
out << *s;
}
else
{
unsigned c = static_cast<unsigned char>(*s);
out << '\\' << std::oct << std::setw(3) << std::setfill('0') << c;
}
break;
}
}
out << '"';
return out.str();
}

void reportError(const char *funcName, const char *filename)
{
std::stringstream out;
out << "Error: " << funcName << "(" << encode(filename) << ")==nullptr ";
PHYSFS_ErrorCode err = PHYSFS_getLastErrorCode();
out << "(code: " << err << ": " << PHYSFS_getErrorByCode(err) << ") ";

PHYSFS_Stat stat;
if (PHYSFS_stat(filename, &stat))
{
out << "filesize=" << stat.filesize << " filetype=" << stat.filetype << " readonly=" << stat.readonly;
}
else
{
PHYSFS_ErrorCode err = PHYSFS_getLastErrorCode();
out << "PHYSFS_stat() failed (code: " << err << ": " << PHYSFS_getErrorByCode(err) << ")";
}

GetLogger()->Error("%%", out.str());
}
};

PHYSFS_File *LoudOpenWrite(const char *filename)
{
PHYSFS_getLastErrorCode(); // discard previous errors
PHYSFS_File *file = PHYSFS_openWrite(filename);
if (file != nullptr) return file;
reportError("PHYSFS_openWrite", filename);
return nullptr;
}

PHYSFS_File *LoudOpenAppend(const char *filename)
{
PHYSFS_getLastErrorCode(); // discard previous errors
PHYSFS_File *file = PHYSFS_openAppend(filename);
if (file != nullptr) return file;
reportError("PHYSFS_openAppend", filename);
return nullptr;
}

PHYSFS_File *LoudOpenRead(const char *filename)
{
PHYSFS_getLastErrorCode(); // discard previous errors
PHYSFS_File *file = PHYSFS_openRead(filename);
if (file != nullptr) return file;
reportError("PHYSFS_openRead", filename);
return nullptr;
}
5 changes: 5 additions & 0 deletions colobot-base/src/common/resources/physfs_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#include <physfs.h>

PHYSFS_File *LoudOpenWrite(const char *filename);
PHYSFS_File *LoudOpenAppend(const char *filename);
PHYSFS_File *LoudOpenRead(const char *filename);
3 changes: 2 additions & 1 deletion colobot-base/src/common/resources/resourcemanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "common/config.h"
#include "common/logger.h"
#include "common/stringutils.h"
#include "common/resources/physfs_utils.h"

#include <algorithm>
#include <physfs.h>
Expand Down Expand Up @@ -233,7 +234,7 @@ long long CResourceManager::GetFileSize(const std::filesystem::path& filename)
{
if (PHYSFS_isInit())
{
PHYSFS_File* file = PHYSFS_openRead(CleanPath(filename).c_str());
PHYSFS_File* file = LoudOpenRead(CleanPath(filename).c_str());
if(file == nullptr) return -1;
long long size = PHYSFS_fileLength(file);
PHYSFS_close(file);
Expand Down
3 changes: 2 additions & 1 deletion colobot-base/src/common/resources/sdl_file_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "common/logger.h"
#include "common/stringutils.h"
#include "common/resources/physfs_utils.h"

#include <physfs.h>

Expand Down Expand Up @@ -140,7 +141,7 @@ CSDLFileWrapper::CSDLFileWrapper(const std::filesystem::path& filename)

auto path = StrUtils::ToString(filename.lexically_normal());

PHYSFS_File *file = PHYSFS_openRead(path.c_str());
PHYSFS_File *file = LoudOpenRead(path.c_str());
if (file == nullptr)
{
GetLogger()->Error("Error opening file with PHYSFS: \"%%\"", filename);
Expand Down
3 changes: 2 additions & 1 deletion colobot-base/src/common/resources/sdl_memory_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "common/logger.h"
#include "common/stringutils.h"
#include "common/resources/physfs_utils.h"

#include <physfs.h>

Expand All @@ -37,7 +38,7 @@ CSDLMemoryWrapper::CSDLMemoryWrapper(const std::filesystem::path& filename)
return;
}

PHYSFS_File *file = PHYSFS_openRead(StrUtils::ToString(filename.lexically_normal()).c_str());
PHYSFS_File *file = LoudOpenRead(StrUtils::ToString(filename.lexically_normal()).c_str());
if (file == nullptr)
{
GetLogger()->Error("Error opening file with PHYSFS: \"%%\"", filename);
Expand Down
6 changes: 3 additions & 3 deletions colobot-base/src/common/resources/sndfile_wrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "common/resources/sndfile_wrapper.h"

#include "common/stringutils.h"
#include "common/resources/physfs_utils.h"

#include <cstring>

Expand Down Expand Up @@ -72,7 +73,7 @@ CSNDFileWrapper::CSNDFileWrapper(const std::filesystem::path& filename)
m_snd_callbacks = { SNDLength, SNDSeek, SNDRead, SNDWrite, SNDTell };
if (PHYSFS_isInit())
{
m_file = PHYSFS_openRead(StrUtils::ToString(filename).c_str());
m_file = LoudOpenRead(StrUtils::ToString(filename).c_str());
}
else
{
Expand All @@ -88,8 +89,7 @@ CSNDFileWrapper::CSNDFileWrapper(const std::filesystem::path& filename)
}
else
{
PHYSFS_ErrorCode errorCode = PHYSFS_getLastErrorCode();
m_last_error = std::string(PHYSFS_getErrorByCode(errorCode));
m_last_error = "Error in PHYSFS_openRead()";
}
}

Expand Down
5 changes: 3 additions & 2 deletions colobot-base/src/graphics/opengl33/glutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "common/image.h"
#include "common/logger.h"
#include "common/resources/physfs_utils.h"

#include <SDL.h>
#include <physfs.h>
Expand Down Expand Up @@ -325,7 +326,7 @@ std::string GetLastShaderError()

std::string LoadSource(const std::string& path)
{
PHYSFS_file* file = PHYSFS_openRead(path.c_str());
PHYSFS_file* file = LoudOpenRead(path.c_str());
if (file == nullptr)
{
GetLogger()->Error("Cannot read shader source file");
Expand Down Expand Up @@ -380,7 +381,7 @@ GLint CreateShader(GLint type, const std::vector<std::string>& sources)

GLint LoadShader(GLint type, const char* filename)
{
PHYSFS_file *file = PHYSFS_openRead(filename);
PHYSFS_file *file = LoudOpenRead(filename);
if (file == nullptr)
{
GetLogger()->Error("Cannot read shader source file");
Expand Down

0 comments on commit 8994b0c

Please sign in to comment.