From bd857d24443b8c8f5d2f58047c3f8ac5f058acea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Ankarstr=C3=B6m?= Date: Wed, 17 Aug 2022 02:33:26 +0200 Subject: Make FileView more type-safe. --- c/data.cpp | 64 -------------------------------- c/data.h | 100 +++++++++++++++++++++++++++++++++++++++++++------- c/debug.cpp | 4 +- c/episodelistview.cpp | 16 ++++---- c/episodelistview.h | 2 +- c/test.cpp | 59 ++++++++++++++--------------- c/wcharptr.cpp | 2 +- c/win.cpp | 2 +- c/win.h | 4 +- 9 files changed, 132 insertions(+), 121 deletions(-) (limited to 'c') diff --git a/c/data.cpp b/c/data.cpp index 60decc9..9dd6ef8 100644 --- a/c/data.cpp +++ b/c/data.cpp @@ -1,65 +1 @@ -#include -#include - #include "data.h" -#include "win.h" - -template -inline unsigned char* ValToBuf(const T& val, unsigned char* const buf) -{ - memcpy(buf, &val, sizeof(val)); - return buf+sizeof(val); -} - -template -inline unsigned char* BufToVal(unsigned char* const buf, T& val) -{ - memcpy(&val, buf, sizeof(val)); - return buf+sizeof(val); -} - -FileView::FileView(const wchar_t* const filename, const size_t cb) -{ - hf = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, - 0, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); - if (hf == INVALID_HANDLE_VALUE) { - if (GetLastError() == ERROR_FILE_NOT_FOUND) { - hf = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, - 0, nullptr, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, nullptr); - if (hf == INVALID_HANDLE_VALUE) - throw Win32Error{}; - } else - throw Win32Error{}; - } - - LARGE_INTEGER cbMap; - cbMap.QuadPart = cb; - hm = CreateFileMapping(hf, nullptr, PAGE_READWRITE, - cbMap.HighPart, cbMap.LowPart, nullptr); - if (!hm) - throw Win32Error{}; - - view = reinterpret_cast(MapViewOfFile(hm, FILE_MAP_ALL_ACCESS, 0, 0, 0)); - if (!view) - throw Win32Error{}; -} - -FileView::~FileView() -{ - FlushViewOfFile(view, 0); - UnmapViewOfFile(view); - CloseHandle(hm); - CloseHandle(hf); -} - -void Write(unsigned char* buf, const ElvDataA& e) -{ - memcpy(buf, reinterpret_cast(&e), sizeof(e)); -} - -ElvDataA* Read(unsigned char* const buf) -{ - if (buf[0] != 'a') - return nullptr; - return reinterpret_cast(buf); -} diff --git a/c/data.h b/c/data.h index 2cc4c14..a93b256 100644 --- a/c/data.h +++ b/c/data.h @@ -1,15 +1,21 @@ #ifndef DATA_H #define DATA_H +#include + #include "pl.h" #include "util.h" #include "wcharptr.h" +#include "win.h" + +/* The structs ending with A are written as-is to disk. As such, they + * should be regarded as immutable. If the format needs to be changed + * in the future, then new structs ending with B should be added. */ -/* ElvDataA and DlvDataA are written as-is to disk; note the careful - * alignment. As such, they should be regarded as immutable. If the - * format needs to be changed in the future, then new structs called - * ElvDataB and DlvDataB should be added. */ +/* Note that unsigned chars are 1-byte-aligned, wchar_t are + * 2-byte-aligned and ints are 4-byte-aligned (on x86 Windows). */ +/* Basic episode data presented in episode list view. */ struct ElvDataA { unsigned char version = 'a'; @@ -21,6 +27,7 @@ struct ElvDataA wchar_t title[128] = {0}; }; +/* Extra episode data presented in data list view. */ struct DlvDataA { wchar_t date[32] = {0}; @@ -30,21 +37,86 @@ struct DlvDataA wchar_t wiki[192] = {0}; }; +/* Variable template for obtaining the version of a given struct. */ +template +constexpr inline unsigned char Version = T{}.version; + +template +constexpr inline bool HasVersion = false; + +template +constexpr inline bool HasVersion> = true; + +/* FileView objects manage a memory-mapped file. The view buffer may + * be treated as an array of a given type T. Note that reading and + * writing a view can throw structured exceptions. We ignore these. */ +template struct FileView { - FileView(const wchar_t* filename, size_t cb); - ~FileView(); - inline operator unsigned char*() { return view; } - inline operator ElvDataA*() { return reinterpret_cast(view); } + FileView(const wchar_t* filename, size_t c_) : c(c_) + { + hf = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, + 0, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + if (hf == INVALID_HANDLE_VALUE) { + if (GetLastError() == ERROR_FILE_NOT_FOUND) { + hf = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, + 0, nullptr, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, nullptr); + if (hf == INVALID_HANDLE_VALUE) + throw Win32Error{}; + } else + throw Win32Error{}; + } + + LARGE_INTEGER cbMap; + cbMap.QuadPart = c*sizeof(T); + hm = CreateFileMapping(hf, nullptr, PAGE_READWRITE, + cbMap.HighPart, cbMap.LowPart, nullptr); + if (!hm) + throw Win32Error{}; + + view = reinterpret_cast(MapViewOfFile(hm, FILE_MAP_ALL_ACCESS, 0, 0, 0)); + if (!view) + throw Win32Error{}; + } + + ~FileView() + { + FlushViewOfFile(view, 0); + UnmapViewOfFile(view); + CloseHandle(hm); + CloseHandle(hf); + } + + /* Access element by index, performing bounds check and, if + * applicable for T, version validation. */ + T& At(size_t i) + { + if (i >= c) + throw std::out_of_range("index larger than buffer"); + + T& t = view[i]; + + if constexpr (HasVersion) + if (t.version != Version) + throw std::runtime_error("invalid struct version"); + + /* TODO: Use a custom exception type that informs the + * user of possible data corruption. */ + + return t; + } + + inline operator T*() noexcept + { + return view; + } + HANDLE hf; HANDLE hm; - unsigned char* view; - /* TODO: Handle exceptions on read and write... */ + T* view; + size_t c; }; - -void Write(unsigned char* buf, const ElvDataA& e); -ElvDataA* Read(unsigned char* buf); - + inline int FromWeb(const int iEp, ElvDataA& e, DlvDataA& d) noexcept { WcharPtr title, wiki, date, source, hint; diff --git a/c/debug.cpp b/c/debug.cpp index 6b5baff..b23df37 100644 --- a/c/debug.cpp +++ b/c/debug.cpp @@ -19,13 +19,13 @@ Benchmark::Benchmark(const char* const name, const int id, const int avgmax) if (!freq) { static LARGE_INTEGER liFreq; if (!QueryPerformanceFrequency(&liFreq)) - throw Win32Error(); + throw Win32Error{}; freq = liFreq.QuadPart; } LARGE_INTEGER liTicks; if (!QueryPerformanceCounter(&liTicks)) - throw Win32Error(); + throw Win32Error{}; ticks = liTicks.QuadPart; } diff --git a/c/episodelistview.cpp b/c/episodelistview.cpp index 0451132..be753a6 100644 --- a/c/episodelistview.cpp +++ b/c/episodelistview.cpp @@ -12,9 +12,11 @@ #include "util.h" #include "win.h" +constexpr size_t MAX_EPISODES = 8192; + EpisodeListView::EpisodeListView(const HWND hWndParent) : ListView(hWndParent, reinterpret_cast(IDC_EPISODELISTVIEW), 0), - m_fv(L"elvdata.dat", sizeof(ElvDataA)*8192) + m_fv(L"elvdata.dat", MAX_EPISODES) { LVCOLUMN lvc; @@ -55,7 +57,7 @@ void EpisodeListView::HandleContextMenu(const WORD command) LVITEM lvi = {LVIF_PARAM, -1}; while (FindNextItem(&lvi, LVNI_SELECTED)) { - ElvDataA& e = static_cast(m_fv)[lvi.lParam-1]; + ElvDataA& e = m_fv.At(lvi.lParam-1); if (ID_SUBGROUP(command) == IDG_CTX_RATE) { /* Process rate commands. */ @@ -117,7 +119,7 @@ LRESULT EpisodeListView::HandleNotify(const LPARAM lParam) case LVN_GETDISPINFO: /* Display item. */ { NMLVDISPINFO* const nm = reinterpret_cast(lParam); - ElvDataA& e = reinterpret_cast(m_fv.view)[nm->item.lParam-1]; + ElvDataA& e = m_fv.At(nm->item.lParam-1); wchar_t* vs[] = {e.siEp, e.title, e.sRating}; /* ELVSIEPISODE, ELVSITITLE, ELVSIRATING */ nm->item.pszText = vs[nm->item.iSubItem]; return 0; @@ -166,7 +168,7 @@ LRESULT EpisodeListView::HandleNotify(const LPARAM lParam) break; case CDDS_ITEMPREPAINT: { - const ElvDataA& e = static_cast(m_fv)[nm->nmcd.lItemlParam-1]; + const ElvDataA& e = m_fv.At(nm->nmcd.lItemlParam-1); if (!e.bWatched) { extern HFONT g_hfBold; Require(SelectObject(nm->nmcd.hdc, g_hfBold)); @@ -329,8 +331,8 @@ int CALLBACK EpisodeListView::SortProc(const LPARAM iItem1, const LPARAM iItem2, * If m_iSortCol is negative, the order is descending. */ const int order = Cmp(elv->m_iSortCol, 0); - const ElvDataA& e1 = static_cast(elv->m_fv)[lvi1.lParam-1]; - const ElvDataA& e2 = static_cast(elv->m_fv)[lvi2.lParam-1]; + const ElvDataA& e1 = elv->m_fv[lvi1.lParam-1]; + const ElvDataA& e2 = elv->m_fv.At(lvi2.lParam-1); switch (abs(elv->m_iSortCol)-1) { case ELVSIEPISODE: @@ -398,7 +400,7 @@ void EpisodeListView::Update() SendMessage(hWnd, WM_SETREDRAW, FALSE, 0); ListView_DeleteAllItems(hWnd); for (int iEp = 1; iEp <= cEp; iEp++) { - ElvDataA e = static_cast(m_fv)[iEp-1]; + ElvDataA e = m_fv.At(iEp-1); if (!e.siEp[0]) continue; diff --git a/c/episodelistview.h b/c/episodelistview.h index 0604d15..f58f3ca 100644 --- a/c/episodelistview.h +++ b/c/episodelistview.h @@ -33,7 +33,7 @@ struct EpisodeListView : public ListView private: int m_iSortCol; static int CALLBACK SortProc(LPARAM lParam1, LPARAM lParam2, LPARAM extra); - FileView m_fv; + FileView m_fv; }; #endif diff --git a/c/test.cpp b/c/test.cpp index cecc408..84e0631 100644 --- a/c/test.cpp +++ b/c/test.cpp @@ -54,40 +54,42 @@ TESTS FromProlog(6, e1_0); FromProlog(10, e2_0); { - FileView fv{L"tmp.dat", sizeof(ElvDataA)*2}; - Write(fv, e1_0); - Write(reinterpret_cast(static_cast(fv)+1), e2_0); + FileView fv{L"tmp.dat", 2}; + memcpy(fv+0, &e1_0, sizeof(e1_0)); + memcpy(fv+1, &e2_0, sizeof(e2_0)); } { - FileView fv{L"tmp.dat", sizeof(ElvDataA)}; - ElvDataA* e1 = Read(fv); - if (e1_0.rating != e1->rating) - FAIL("rating is different (%d/%d)", e1_0.rating, e1->rating); - if (e1_0.bWatched != e1->bWatched) + FileView fv{L"tmp.dat", 2}; + const ElvDataA& e1 = *fv; + if (e1.version != 'a') + FAIL("version is different"); + if (e1_0.rating != e1.rating) + FAIL("rating is different (%d/%d)", e1_0.rating, e1.rating); + if (e1_0.bWatched != e1.bWatched) FAIL("bWatched is different"); - if (e1_0.bTVOriginal != e1->bTVOriginal) + if (e1_0.bTVOriginal != e1.bTVOriginal) FAIL("bTVOriginal is different"); - if (wcscmp(e1_0.sRating, e1->sRating) != 0) + if (wcscmp(e1_0.sRating, e1.sRating) != 0) FAIL("sRating is different"); - if (wcscmp(e1_0.siEp, e1->siEp) != 0) + if (wcscmp(e1_0.siEp, e1.siEp) != 0) FAIL("siEp is different"); - if (wcscmp(e1_0.title, e1->title) != 0) + if (wcscmp(e1_0.title, e1.title) != 0) FAIL("title is different"); } { - FileView fv{L"tmp.dat", sizeof(ElvDataA)*2}; - ElvDataA* e2 = Read(reinterpret_cast(static_cast(fv)+1)); - if (e2_0.rating != e2->rating) - FAIL("rating is different (%d/%d)", e2_0.rating, e2->rating); - if (e2_0.bWatched != e2->bWatched) + FileView fv{L"tmp.dat", 2*sizeof(ElvDataA)}; + ElvDataA& e2 = *reinterpret_cast(&fv.At(sizeof(ElvDataA))); + if (e2_0.rating != e2.rating) + FAIL("rating is different (%d/%d)", e2_0.rating, e2.rating); + if (e2_0.bWatched != e2.bWatched) FAIL("bWatched is different"); - if (e2_0.bTVOriginal != e2->bTVOriginal) + if (e2_0.bTVOriginal != e2.bTVOriginal) FAIL("bTVOriginal is different"); - if (wcscmp(e2_0.sRating, e2->sRating) != 0) + if (wcscmp(e2_0.sRating, e2.sRating) != 0) FAIL("sRating is different"); - if (wcscmp(e2_0.siEp, e2->siEp) != 0) + if (wcscmp(e2_0.siEp, e2.siEp) != 0) FAIL("siEp is different"); - if (wcscmp(e2_0.title, e2->title) != 0) + if (wcscmp(e2_0.title, e2.title) != 0) FAIL("title is different"); } //DeleteFile(L"tmp.dat"); @@ -100,20 +102,19 @@ TESTS return; { - FileView fv{L"tmp.dat", sizeof(ElvDataA)*8192}; - unsigned char* p = fv; + FileView fv{L"tmp.dat", 8192}; + ElvDataA* p = fv; for (int iEp = 1; iEp <= cEp; iEp++) { ElvDataA e; FromProlog(iEp, e); - Write(p, e); - p += sizeof(e); + memcpy(p, &e, sizeof(e)); + p++; } } { - FileView fv{L"tmp.dat", sizeof(ElvDataA)*8192}; - ElvDataA* ve = reinterpret_cast(fv.view); - ElvDataA& e = ve[9]; + FileView fv{L"tmp.dat", 8192}; + ElvDataA& e = fv.At(9); if (wcscmp(e.title, L"Pro Soccer Player Blackmail Case") != 0) FAIL("title is not correct"); } @@ -128,7 +129,7 @@ int RunTests() //EpisodeDataFromWeb{}, EpisodeDataFromProlog{}, IO{}, - MigrateElvDataFromPrologToDisk{}, + //MigrateElvDataFromPrologToDisk{}, }; printf("Results (%llu tests):\n", sizeof(tests)/sizeof(*tests)); diff --git a/c/wcharptr.cpp b/c/wcharptr.cpp index f659767..f7cb948 100644 --- a/c/wcharptr.cpp +++ b/c/wcharptr.cpp @@ -11,7 +11,7 @@ WcharPtr WcharPtr::FromNarrow(const char* const src, const int cp) wchar_t* dst = new wchar_t[cchWide]; if (!MultiByteToWideChar(cp, 0, src, cchNarrow, dst, cchWide)) { delete dst; - throw Win32Error(); + throw Win32Error{}; } return dst; } diff --git a/c/win.cpp b/c/win.cpp index 95ec9a7..28dba27 100644 --- a/c/win.cpp +++ b/c/win.cpp @@ -145,7 +145,7 @@ Library::Library(const wchar_t* const lib) { m_hModule = LoadLibrary(lib); if (!m_hModule) - throw Win32Error(); + throw Win32Error{}; } Library::~Library() diff --git a/c/win.h b/c/win.h index e9a6839..a77362f 100644 --- a/c/win.h +++ b/c/win.h @@ -53,7 +53,7 @@ T* Library::GetProcAddress(const char* const szProc) noexcept template inline T Require(const T x) { - if (!x) throw Win32Error(); + if (!x) throw Win32Error{}; return x; } @@ -62,7 +62,7 @@ template inline T Prefer(const T x) { if (!x) { - EBMessageBox(Win32Error().What(), L"System Error", MB_ICONWARNING); + EBMessageBox(Win32Error{}.What(), L"System Error", MB_ICONWARNING); return (T)0; } return x; -- cgit v1.2.3