Skip to content

Commit 6603d7c

Browse files
zaozao
committed
feat: make images and file streams use fs::path
This change adds some foundational changes to widen the rest of the runtime to use `std::filesystem::path` and try to detect misuse where narrow strings are used as-is. Dependent code in the codebase will fail to build with this change.
1 parent 1fe8c77 commit 6603d7c

File tree

4 files changed

+92
-49
lines changed

4 files changed

+92
-49
lines changed

‎engine/common/streams.cpp‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,14 @@ bool fileInputStream_c::Read(void* out, size_t len)
255255
return fread(out, len, 1, file) < 1;
256256
}
257257

258-
bool fileInputStream_c::FileOpen(const char* fileName, bool binary)
258+
bool fileInputStream_c::FileOpen(std::filesystem::path const& fileName, bool binary)
259259
{
260260
FileClose();
261-
file = fopen(fileName, binary? "rb" : "r");
261+
#ifdef _WIN32
262+
file = _wfopen(fileName.c_str(), binary ? L"rb" : L"r");
263+
#else
264+
file = fopen(fileName.c_str(), binary ? "rb" : "r");
265+
#endif
262266
if ( !file ) {
263267
return true;
264268
}
@@ -277,10 +281,14 @@ bool fileOutputStream_c::Write(const void* in, size_t len)
277281
return fwrite(in, len, 1, file) < 1;
278282
}
279283

280-
bool fileOutputStream_c::FileOpen(const char* fileName, bool binary)
284+
bool fileOutputStream_c::FileOpen(std::filesystem::path const& fileName, bool binary)
281285
{
282286
FileClose();
283-
file = fopen(fileName, binary? "wb" : "w");
287+
#ifdef _WIN32
288+
file = _wfopen(fileName.c_str(), binary ? L"wb" : L"w");
289+
#else
290+
file = fopen(fileName.c_str(), binary ? "wb" : "w");
291+
#endif
284292
if ( !file ) {
285293
return true;
286294
}

‎engine/common/streams.h‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
// Streams Header
55
//
66

7+
#include <filesystem>
8+
#include <string>
9+
#include <string_view>
10+
711
// ==========
812
// Base Class
913
// ==========
@@ -94,13 +98,29 @@ class fileStreamBase_c: public ioStream_c {
9498
class fileInputStream_c: public fileStreamBase_c {
9599
public:
96100
bool Read(void* out, size_t len);
97-
bool FileOpen(const char* fileName, bool binary);
101+
102+
// Force compile error on narrow strings to favour `std::filesystem::path`.
103+
// These are unfortunately necessary as the path constructor is eager to
104+
// interpret narrow strings as the ACP codepage. Prefer using
105+
// `std::filesystem::u8path` (C++17) or `std::u8string` (since C++20) in
106+
// the calls to these functions.
107+
bool FileOpen(char const*, bool) = delete;
108+
bool FileOpen(std::string const&, bool) = delete;
109+
bool FileOpen(std::string_view*, bool) = delete;
110+
111+
bool FileOpen(std::filesystem::path const& fileName, bool binary);
98112
};
99113

100114
class fileOutputStream_c: public fileStreamBase_c {
101115
public:
102116
bool Write(const void* in, size_t len);
103-
bool FileOpen(const char* fileName, bool binary);
117+
118+
// Force compile error like in `fileInputStream_c` on non-path types.
119+
bool FileOpen(char const*, bool) = delete;
120+
bool FileOpen(std::string const&, bool) = delete;
121+
bool FileOpen(std::string_view*, bool) = delete;
122+
123+
bool FileOpen(std::filesystem::path const& fileName, bool binary);
104124
void FilePrintf(const char* fmt, ...);
105125
void FileFlush();
106126
};

‎engine/core/core_image.cpp‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -79,39 +79,39 @@ void image_c::Free()
7979
tex = {};
8080
}
8181

82-
bool image_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallback)
82+
bool image_c::Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback)
8383
{
8484
return true; // o_O
8585
}
8686

87-
bool image_c::Save(const char* fileName)
87+
bool image_c::Save(std::filesystem::path const& fileName)
8888
{
8989
return true; // o_O
9090
}
9191

92-
image_c* image_c::LoaderForFile(IConsole* conHnd, const char* fileName)
92+
image_c* image_c::LoaderForFile(IConsole* conHnd, std::filesystem::path const& fileName)
9393
{
94+
auto nameU8 = fileName.u8string();
9495
fileInputStream_c in;
9596
if (in.FileOpen(fileName, true)) {
96-
conHnd->Warning("'%s' doesn't exist or cannot be opened", fileName);
97+
conHnd->Warning("'%s' doesn't exist or cannot be opened", nameU8.c_str());
9798
return NULL;
9899
}
99100

100101
// Detect first by extension, as decompressing could be expensive.
101-
auto p = std::filesystem::path(fileName); // NOTE(LV): This should be u8path later.
102-
if (p.extension() == ".zst") {
103-
auto inner = p.filename();
102+
if (fileName.extension() == ".zst") {
103+
auto inner = fileName.filename();
104104
inner.replace_extension();
105105
if (inner.extension() == ".dds")
106106
return new dds_c(conHnd);
107107
}
108-
if (p.extension() == ".dds")
108+
if (fileName.extension() == ".dds")
109109
return new dds_c(conHnd);
110110

111-
// Attempt to detect image file type from first 12 bytes of file
111+
// Attempt to detect image file type from first 4 bytes of file
112112
byte dat[4];
113113
if (in.Read(dat, 4)) {
114-
conHnd->Warning("'%s': cannot read image file (file is corrupt?)", fileName);
114+
conHnd->Warning("'%s': cannot read image file (file is corrupt?)", nameU8.c_str());
115115
return NULL;
116116
}
117117
if (dat[0] == 0xFF && dat[1] == 0xD8) {
@@ -130,7 +130,7 @@ image_c* image_c::LoaderForFile(IConsole* conHnd, const char* fileName)
130130
// Detect all valid image types (whether supported or not)
131131
return new targa_c(conHnd);
132132
}
133-
conHnd->Warning("'%s': unsupported image file format", fileName);
133+
conHnd->Warning("'%s': unsupported image file format", nameU8.c_str());
134134
return NULL;
135135
}
136136

@@ -153,24 +153,24 @@ struct tgaHeader_s {
153153
};
154154
#pragma pack(pop)
155155

156-
bool targa_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallback)
156+
bool targa_c::Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback)
157157
{
158-
Free();
159-
160158
// Open the file
161159
fileInputStream_c in;
162160
if (in.FileOpen(fileName, true)) {
163161
return true;
164162
}
165163

164+
auto nameU8 = fileName.u8string();
165+
166166
// Read header
167167
tgaHeader_s hdr;
168168
if (in.TRead(hdr)) {
169-
con->Warning("TGA '%s': couldn't read header", fileName);
169+
con->Warning("TGA '%s': couldn't read header", nameU8.c_str());
170170
return true;
171171
}
172172
if (hdr.colorMapType) {
173-
con->Warning("TGA '%s': color mapped images not supported", fileName);
173+
con->Warning("TGA '%s': color mapped images not supported", nameU8.c_str());
174174
return true;
175175
}
176176
in.Seek(hdr.idLen, SEEK_CUR);
@@ -188,7 +188,7 @@ bool targa_c::Load(const char* fileName, std::optional<size_callback_t> sizeCall
188188
if (ittable[it_m][0] == (hdr.imgType & 7) && ittable[it_m][1] == hdr.depth) break;
189189
}
190190
if (it_m == 3) {
191-
con->Warning("TGA '%s': unsupported image type (it: %d pd: %d)", fileName, hdr.imgType, hdr.depth);
191+
con->Warning("TGA '%s': unsupported image type (it: %d pd: %d)", nameU8.c_str(), hdr.imgType, hdr.depth);
192192
return true;
193193
}
194194

@@ -211,7 +211,7 @@ bool targa_c::Load(const char* fileName, std::optional<size_callback_t> sizeCall
211211
in.TRead(rlehdr);
212212
int rlen = ((rlehdr & 0x7F) + 1) * comp;
213213
if (x + rlen > rowSize) {
214-
con->Warning("TGA '%s': invalid RLE coding (overlong row)", fileName);
214+
con->Warning("TGA '%s': invalid RLE coding (overlong row)", nameU8.c_str());
215215
delete[] dat;
216216
return true;
217217
}
@@ -247,7 +247,7 @@ bool targa_c::Load(const char* fileName, std::optional<size_callback_t> sizeCall
247247
return !CopyRaw(type, width, height, dat);
248248
}
249249

250-
bool targa_c::Save(const char* fileName)
250+
bool targa_c::Save(std::filesystem::path const& fileName)
251251
{
252252
auto format = tex.format();
253253
if (is_compressed(format) || !is_unsigned_integer(format))
@@ -276,7 +276,7 @@ bool targa_c::Save(const char* fileName)
276276
// JPEG Image
277277
// ==========
278278

279-
bool jpeg_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallback)
279+
bool jpeg_c::Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback)
280280
{
281281
Free();
282282

@@ -286,6 +286,8 @@ bool jpeg_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallb
286286
return true;
287287
}
288288

289+
auto nameU8 = fileName.u8string();
290+
289291
std::vector<byte> fileData(in.GetLen());
290292
if (in.Read(fileData.data(), fileData.size())) {
291293
return true;
@@ -295,7 +297,7 @@ bool jpeg_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallb
295297
return true;
296298
}
297299
if (in_comp != 1 && in_comp != 3) {
298-
con->Warning("JPEG '%s': unsupported component count '%d'", fileName, in_comp);
300+
con->Warning("JPEG '%s': unsupported component count '%d'", nameU8.c_str(), in_comp);
299301
return true;
300302
}
301303
if (sizeCallback)
@@ -313,7 +315,7 @@ bool jpeg_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallb
313315
return !success;
314316
}
315317

316-
bool jpeg_c::Save(const char* fileName)
318+
bool jpeg_c::Save(std::filesystem::path const& fileName)
317319
{
318320
// JPEG only supports RGB and grayscale images
319321
auto format = tex.format();
@@ -342,7 +344,7 @@ bool jpeg_c::Save(const char* fileName)
342344
// PNG Image
343345
// =========
344346

345-
bool png_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallback)
347+
bool png_c::Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback)
346348
{
347349
Free();
348350

@@ -380,7 +382,7 @@ bool png_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallba
380382
return !success;
381383
}
382384

383-
bool png_c::Save(const char* fileName)
385+
bool png_c::Save(std::filesystem::path const& fileName)
384386
{
385387
auto format = tex.format();
386388
if (is_compressed(format) || !is_unsigned_integer(format))
@@ -409,7 +411,7 @@ bool png_c::Save(const char* fileName)
409411
// GIF Image
410412
// =========
411413

412-
bool gif_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallback)
414+
bool gif_c::Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback)
413415
{
414416
// Open file
415417
fileInputStream_c in;
@@ -440,7 +442,7 @@ bool gif_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallba
440442
}
441443
}
442444

443-
bool gif_c::Save(const char* fileName)
445+
bool gif_c::Save(std::filesystem::path const& fileName)
444446
{
445447
// HELL no.
446448
return true;
@@ -450,9 +452,8 @@ bool gif_c::Save(const char* fileName)
450452
// DDS Image
451453
// =========
452454

453-
bool dds_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallback)
455+
bool dds_c::Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback)
454456
{
455-
auto p = std::filesystem::path(fileName); // NOTE(LV): This should be u8path later.
456457
// Open file
457458
fileInputStream_c in;
458459
if (in.FileOpen(fileName, true))
@@ -462,7 +463,7 @@ bool dds_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallba
462463
if (in.Read(fileData.data(), fileData.size()))
463464
return true;
464465

465-
if (p.extension() == ".zst" || fileData.size() >= 4 && *(uint32_t*)fileData.data() == 0xFD2FB528) {
466+
if (fileName.extension() == ".zst" || fileData.size() >= 4 && *(uint32_t*)fileData.data() == 0xFD2FB528) {
466467
auto ret = DecompressZstandard(as_bytes(gsl::span(fileData)));
467468
if (!ret.has_value())
468469
return true;
@@ -476,7 +477,7 @@ bool dds_c::Load(const char* fileName, std::optional<size_callback_t> sizeCallba
476477
return false;
477478
}
478479

479-
bool dds_c::Save(const char* fileName)
480+
bool dds_c::Save(std::filesystem::path const& fileName)
480481
{
481482
// Nope.
482483
return true;

‎engine/core/core_image.h‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,53 +54,67 @@ class image_c {
5454

5555
using size_callback_t = std::function<void(int, int)>;
5656

57-
virtual bool Load(const char* fileName, std::optional<size_callback_t> sizeCallback = {});
58-
virtual bool Save(const char* fileName);
57+
virtual bool Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback = {});
58+
virtual bool Save(std::filesystem::path const& fileName);
5959

6060
bool CopyRaw(int type, dword width, dword height, const byte* dat);
6161
void Free();
6262

63-
static image_c* LoaderForFile(IConsole* conHnd, const char* fileName);
63+
static image_c* LoaderForFile(IConsole* conHnd, char const* fileName) = delete;
64+
static image_c* LoaderForFile(IConsole* conHnd, std::filesystem::path const& fileName);
65+
66+
private:
67+
// Force compile error on narrow strings to favour `std::filesystem::path`.
68+
// These are unfortunately necessary as the path constructor is eager to
69+
// interpret narrow strings as the ACP codepage. Prefer using
70+
// `std::filesystem::u8path` (C++17) or `std::u8string` (since C++20) in
71+
// the calls to these functions.
72+
virtual bool Load(char const* fileName) { return true; }
73+
virtual bool Load(std::string const& fileName) { return true; }
74+
virtual bool Load(std::string_view fileName) { return true; }
75+
virtual bool Save(char const* fileName) { return true; }
76+
virtual bool Save(std::string const& fileName) { return true; }
77+
virtual bool Save(std::string_view fileName) { return true; }
6478
};
6579

6680
// Targa Image
6781
class targa_c : public image_c {
6882
public:
6983
bool rle;
7084
targa_c(IConsole* conHnd) : image_c(conHnd) { rle = true; }
71-
bool Load(const char* fileName, std::optional<size_callback_t> sizeCallback = {}) override;
72-
bool Save(const char* fileName) override;
85+
bool Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback = {}) override;
86+
bool Save(std::filesystem::path const& fileName) override;
7387
};
7488

7589
// JPEG Image
7690
class jpeg_c : public image_c {
7791
public:
7892
int quality;
7993
jpeg_c(IConsole* conHnd) : image_c(conHnd) { quality = 80; }
80-
bool Load(const char* fileName, std::optional<size_callback_t> sizeCallback = {}) override;
81-
bool Save(const char* fileName) override;
94+
bool Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback = {}) override;
95+
bool Save(std::filesystem::path const& fileName) override;
8296
};
8397

8498
// PNG Image
8599
class png_c : public image_c {
86100
public:
87101
png_c(IConsole* conHnd) : image_c(conHnd) { }
88-
bool Load(const char* fileName, std::optional<size_callback_t> sizeCallback = {}) override;
89-
bool Save(const char* fileName) override;
102+
bool Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback = {}) override;
103+
bool Save(std::filesystem::path const& fileName) override;
90104
};
91105

92106
// GIF Image
93107
class gif_c : public image_c {
94108
public:
95109
gif_c(IConsole* conHnd) : image_c(conHnd) { }
96-
bool Load(const char* fileName, std::optional<size_callback_t> sizeCallback = {}) override;
97-
bool Save(const char* fileName) override;
110+
bool Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback = {}) override;
111+
bool Save(std::filesystem::path const& fileName) override;
98112
};
99113

100114
// DDS Image
101115
class dds_c : public image_c {
102116
public:
103117
dds_c(IConsole* conHnd) : image_c(conHnd) {}
104-
bool Load(const char* fileName, std::optional<size_callback_t> sizeCallback = {}) override;
105-
bool Save(const char* fileName) override;
118+
bool Load(std::filesystem::path const& fileName, std::optional<size_callback_t> sizeCallback = {}) override;
119+
bool Save(std::filesystem::path const& fileName) override;
106120
};

0 commit comments

Comments
 (0)