From 8b4209ec0b948f8188cc7593ba04b1787827c51d Mon Sep 17 00:00:00 2001 From: Victor Derks Date: Fri, 4 Oct 2024 16:10:06 +0200 Subject: [PATCH] Add buffered_stream_reader_test and fix registry - Add some tests for buffered_stream_reader - Fix the registration for .pgm files (was overwritten by .ppm) --- spelling.dic | 5 ++ src/buffered_stream_reader.cpp | 115 +++++++++++---------------- src/buffered_stream_reader.ixx | 12 +-- src/dll_main.cpp | 6 +- src/pnm_header.ixx | 9 +-- test/buffered_stream_reader_test.cpp | 67 ++++++++++++++++ test/test.vcxproj | 1 + test/test.vcxproj.filters | 3 + 8 files changed, 135 insertions(+), 83 deletions(-) create mode 100644 test/buffered_stream_reader_test.cpp diff --git a/spelling.dic b/spelling.dic index b3f74b2..2fc5066 100644 --- a/spelling.dic +++ b/spelling.dic @@ -14,3 +14,8 @@ derks bcfd Anymap +jpegls +pgmfile +ppmfile +graymap +pixmap diff --git a/src/buffered_stream_reader.cpp b/src/buffered_stream_reader.cpp index d63c522..c079e31 100644 --- a/src/buffered_stream_reader.cpp +++ b/src/buffered_stream_reader.cpp @@ -3,6 +3,7 @@ module; +#include "macros.hpp" #include "intellisense.hpp" module buffered_stream_reader; @@ -19,6 +20,8 @@ constexpr UINT MAX_BUFFER_SIZE = 65536; buffered_stream_reader::buffered_stream_reader(_In_ IStream* stream) : buffer_(MAX_BUFFER_SIZE) { + ASSERT(stream); + stream_.copy_from(stream); // buffer_ = new BYTE[MAX_BUFFER_SIZE]; @@ -29,66 +32,70 @@ buffered_stream_reader::buffered_stream_reader(_In_ IStream* stream) : buffer_(M buffer_size_ = read; } -HRESULT buffered_stream_reader::ReadChar(char* c) +uint32_t buffered_stream_reader::read_int() +{ + char str[12]; + + read_string(str, sizeof(str)); + uint32_t value; + if (const auto [ptr, ec] = std::from_chars(str, std::end(str), value); ec != std::errc()) + winrt::throw_hresult(WINCODEC_ERR_BADSTREAMDATA); + + return value; +} + +bool buffered_stream_reader::try_read_bytes(void* buffer, const size_t size) +{ + ULONG bytes_read; + read_bytes(buffer, static_cast(size), &bytes_read); + if (bytes_read != size) + return false; + + return true; +} + +char buffered_stream_reader::read_char() { if (position_ + sizeof(char) > buffer_size_) + { if (buffer_size_ == MAX_BUFFER_SIZE) { RefillBuffer(); if (buffer_size_ < sizeof(char)) - return S_FALSE; + winrt::throw_hresult(WINCODEC_ERR_BADSTREAMDATA); } else - return S_FALSE; + winrt::throw_hresult(WINCODEC_ERR_BADSTREAMDATA); + } - *c = buffer_[position_]; + const char result = buffer_[position_]; position_ += sizeof(char); - return S_OK; + return result; } -HRESULT buffered_stream_reader::SkipLine() +void buffered_stream_reader::skip_line() { while (true) { - char c; - - const HRESULT hr = ReadChar(&c); - - if (hr != S_OK) - return hr; + const char c = read_char(); if (c == 0x0A) - return S_OK; + return; } } -HRESULT buffered_stream_reader::ReadString(char* str, ULONG maxCount) +void buffered_stream_reader::read_string(char* str, ULONG maxCount) { - if (maxCount == 0) - return E_INVALIDARG; + ASSERT(maxCount > 0); ULONG charsRead = 0; while (true) { - char c; - - HRESULT hr = ReadChar(&c); - - if (FAILED(hr)) - return hr; - - if (hr == S_FALSE) - if (charsRead == 0) - return S_FALSE; - else - { - *str = 0x00; - return S_OK; - } + char c = read_char(); if (isspace(c)) if (charsRead == 0) @@ -96,29 +103,18 @@ HRESULT buffered_stream_reader::ReadString(char* str, ULONG maxCount) else { *str = 0x00; - return S_OK; + return; } if (c == '#') if (charsRead == 0) { - hr = SkipLine(); - - if (FAILED(hr)) - return hr; - - if (hr == S_FALSE) - { - *str = 0x00; - return hr; - } - + skip_line(); continue; } else { - *str = 0x00; - return S_OK; + winrt::throw_hresult(WINCODEC_ERR_BADSTREAMDATA); } *str = c; @@ -126,31 +122,10 @@ HRESULT buffered_stream_reader::ReadString(char* str, ULONG maxCount) charsRead++; if (charsRead == maxCount) - return WINCODEC_ERR_BADSTREAMDATA; + winrt::throw_hresult(WINCODEC_ERR_BADSTREAMDATA); } } -uint32_t buffered_stream_reader::read_int_slow() -{ - char str[12]; - - winrt::check_hresult(ReadString(str, sizeof(str))); - uint32_t value; - if (const auto [ptr, ec] = std::from_chars(str, std::end(str), value); ec != std::errc()) - winrt::throw_hresult(WINCODEC_ERR_BADSTREAMDATA); - - return value; -} - -bool buffered_stream_reader::try_read_bytes(void* buffer, const size_t size) -{ - ULONG bytes_read; - if (const HRESULT hr{ReadBytes(buffer, static_cast(size), &bytes_read)}; FAILED(hr) || bytes_read != size) - return false; - - return true; -} - void buffered_stream_reader::read_bytes(void* buffer, size_t size) { const size_t remaining_in_buffer = buffer_size_ - position_; @@ -171,7 +146,7 @@ void buffered_stream_reader::read_bytes(void* buffer, size_t size) wincodec::error_stream_read); } -HRESULT buffered_stream_reader::ReadBytes(void* buf, const ULONG count, ULONG* bytesRead) +void buffered_stream_reader::read_bytes(void* buf, const ULONG count, ULONG* bytesRead) { auto b{static_cast(buf)}; ULONG remaining = count; @@ -183,7 +158,7 @@ HRESULT buffered_stream_reader::ReadBytes(void* buf, const ULONG count, ULONG* b memcpy(b, buffer_.data() + position_, remaining); position_ += remaining; *bytesRead = count; - return S_OK; + return; } memcpy(b, buffer_.data() + position_, buffer_size_ - position_); @@ -196,7 +171,7 @@ HRESULT buffered_stream_reader::ReadBytes(void* buf, const ULONG count, ULONG* b if (buffer_size_ == 0) { *bytesRead = count - remaining; - return S_OK; + return; } } } diff --git a/src/buffered_stream_reader.ixx b/src/buffered_stream_reader.ixx index 76c39b4..d74ebd5 100644 --- a/src/buffered_stream_reader.ixx +++ b/src/buffered_stream_reader.ixx @@ -16,11 +16,8 @@ export class buffered_stream_reader final public: explicit buffered_stream_reader(_In_ IStream* stream); - HRESULT ReadChar(char* c); - HRESULT SkipLine(); - HRESULT ReadString(char* str, ULONG maxCount); - [[nodiscard]] std::uint32_t read_int_slow(); - bool try_read_bytes(void* buffer, size_t size); + [[nodiscard]] std::uint32_t read_int(); + [[nodiscard]] bool try_read_bytes(void* buffer, size_t size); void read_bytes(void* buffer, size_t size); [[nodiscard]] std::vector read_bytes(const size_t size) @@ -30,9 +27,12 @@ public: return bytes; } - HRESULT ReadBytes(void* buf, ULONG count, ULONG* bytesRead); + void read_bytes(void* buf, ULONG count, ULONG* bytesRead); private: + char read_char(); + void skip_line(); + void read_string(char* str, ULONG maxCount); void RefillBuffer(); winrt::com_ptr stream_; diff --git a/src/dll_main.cpp b/src/dll_main.cpp index 9cc675f..52aa94d 100644 --- a/src/dll_main.cpp +++ b/src/dll_main.cpp @@ -65,7 +65,7 @@ void register_general_decoder_settings(const GUID& class_id, const GUID& wic_cat void register_decoder_file_extension(const wchar_t* file_type_name, const wchar_t* file_extension, const wchar_t* mime_type) { - const wstring extension_sub_key{LR"(SOFTWARE\Classes\"s + file_extension + LR"\)"}; + const wstring extension_sub_key{LR"(SOFTWARE\Classes\)"s + file_extension + LR"(\)"}; registry::set_value(extension_sub_key, L"", file_type_name); registry::set_value(extension_sub_key, L"Content Type", mime_type); registry::set_value(extension_sub_key, L"PerceivedType", L"image"); @@ -109,7 +109,7 @@ void register_decoder() // Register the byte pattern that allows WICs to identify files as our image type. register_decoder_pattern(sub_key, 0, array{std::byte{0x50}, std::byte{0x35}}); - register_decoder_pattern(sub_key, 0, array{std::byte{0x50}, std::byte{0x36}}); + register_decoder_pattern(sub_key, 1, array{std::byte{0x50}, std::byte{0x36}}); register_decoder_file_extension(L"pgmfile", L".pgm", L"image/x-portable-graymap"); register_decoder_file_extension(L"ppmfile", L".ppm", L"image/x-portable-pixmap"); @@ -183,6 +183,7 @@ extern "C" __control_entrypoint(DllExport) HRESULT __stdcall DllCanUnloadNow() HRESULT __stdcall DllRegisterServer() try { + TRACE("netpbm-wic-codec::DllRegisterServer\n"); register_decoder(); SHChangeNotify(SHCNE_ASSOCCHANGED, SHCNF_IDLIST, nullptr, nullptr); @@ -198,6 +199,7 @@ catch (...) HRESULT __stdcall DllUnregisterServer() try { + TRACE("netpbm-wic-codec::DllUnregisterServer\n"); // Note: keep the .pgm file registration intact. return unregister(id::netpbm_decoder, CATID_WICBitmapDecoders); } diff --git a/src/pnm_header.ixx b/src/pnm_header.ixx index 06a42ae..488f230 100644 --- a/src/pnm_header.ixx +++ b/src/pnm_header.ixx @@ -55,8 +55,7 @@ export struct pnm_header char magic[2]; ULONG bytesRead; - if (const HRESULT hr{streamReader.ReadBytes((BYTE*)&magic, sizeof(magic), &bytesRead)}; FAILED(hr)) - return hr; + streamReader.read_bytes((BYTE*)&magic, sizeof(magic), &bytesRead); if (bytesRead != sizeof(magic)) throw_hresult(wincodec::error_bad_header); @@ -86,8 +85,8 @@ export struct pnm_header throw_hresult(wincodec::error_bad_header); } - width = streamReader.read_int_slow(); - height = streamReader.read_int_slow(); + width = streamReader.read_int(); + height = streamReader.read_int(); if (width < 1 || height < 1) return WINCODEC_ERR_BADHEADER; @@ -96,7 +95,7 @@ export struct pnm_header if (PnmType != PnmType::Bitmap) { - maxColorValue = streamReader.read_int_slow(); + maxColorValue = streamReader.read_int(); } else maxColorValue = 1; diff --git a/test/buffered_stream_reader_test.cpp b/test/buffered_stream_reader_test.cpp new file mode 100644 index 0000000..c0a766b --- /dev/null +++ b/test/buffered_stream_reader_test.cpp @@ -0,0 +1,67 @@ +// Copyright (c) Victor Derks. +// SPDX-License-Identifier: MIT + +#include "intellisense.hpp" +#include "util.hpp" + +import std; +import "win.h"; +import test.winrt; + +import buffered_stream_reader; + +using std::span; +using winrt::com_ptr; +using namespace Microsoft::VisualStudio::CppUnitTestFramework; + +TEST_CLASS(buffered_stream_reader_test) +{ +public: + TEST_METHOD(read_bytes) // NOLINT + { + std::vector source; + source.push_back(0); + source.push_back(1); + buffered_stream_reader reader(create_memory_stream(source).get()); + + std::vector destination(2); + ULONG bytes_read; + reader.read_bytes(destination.data(), static_cast(destination.size()), &bytes_read); + + Assert::AreEqual(2UL, bytes_read); + } + + TEST_METHOD(read_bytes_not_enough_available) // NOLINT + { + std::vector source; + source.push_back(0); + buffered_stream_reader reader(create_memory_stream(source).get()); + + std::vector destination(2); + ULONG bytes_read; + reader.read_bytes(destination.data(), static_cast(destination.size()), &bytes_read); + + Assert::AreEqual(1UL, bytes_read); + } + + TEST_METHOD(read_int) // NOLINT + { + std::vector source; + std::string str1 = "256 "; + source.insert(source.end(), str1.begin(), str1.end()); + buffered_stream_reader reader(create_memory_stream(source).get()); + + auto value = reader.read_int(); + + Assert::AreEqual(256U, value); + } + +private: + com_ptr create_memory_stream(span source) + { + com_ptr stream; + stream.attach(SHCreateMemStream(reinterpret_cast(source.data()), static_cast(source.size()))); + + return stream; + } +}; diff --git a/test/test.vcxproj b/test/test.vcxproj index 22d2f53..77015f7 100644 --- a/test/test.vcxproj +++ b/test/test.vcxproj @@ -167,6 +167,7 @@ + diff --git a/test/test.vcxproj.filters b/test/test.vcxproj.filters index 136823a..50df63f 100644 --- a/test/test.vcxproj.filters +++ b/test/test.vcxproj.filters @@ -45,6 +45,9 @@ Source Files + + Source Files +