From 8b25418e60f50601bc718f5b811ca20bc08e9d3a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 22:31:54 +0000 Subject: [PATCH 1/4] test(edges): NULL, overflow and boundary cases for the untaken branches A new test file aimed at branch (C1) coverage: each NULL argument of the row, resource and block-settings functions and of decode_mask on its own; counts at the edge of overflow (a size_t count past UINT32_MAX, 0xFFFFFFFF guides, 0xFFFFFFFF UTF-16 units); empty lists, cursors outside a list, buffers one byte short; the float helpers at infinity, NaN and DBL_MAX, and channel ids mapped to slots at the limits. Branches 95.7% -> 96.9%; builders.h, views.h, detail/floats.h at 100%. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019egXEpyVAm2D3tvQn2Xcuv --- tests/CMakeLists.txt | 3 +- tests/test_edges.cpp | 315 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+), 1 deletion(-) create mode 100644 tests/test_edges.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ca63b82..05ac779 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -85,7 +85,8 @@ set(_minipsd_test_sources test_composite.cpp test_writer.cpp test_reader.cpp - minipsd_c_smoke.c # minipsd.h compiled as C, compared byte for byte with the C++ build + test_edges.cpp # NULLs, overflowing counts, empty inputs and limits: the branches left over + minipsd_c_smoke.c # minipsd.h compiled as C, compared byte for byte with the C++ build test_alloc_failure.cpp # minipsd.h with an allocator that fails on demand test_golden.cpp # the bytes of 0.1.0, by SHA-256, for a fixed set of documents ) diff --git a/tests/test_edges.cpp b/tests/test_edges.cpp new file mode 100644 index 0000000..bdb99f2 --- /dev/null +++ b/tests/test_edges.cpp @@ -0,0 +1,315 @@ +/** + * \file + * \brief Edge cases across the modules, aimed at the branches the other files leave untaken (C1): + * each NULL argument on its own, counts and sizes at the edge of overflow, empty inputs, + * cursors outside a list, buffers one byte short, and the float and slot helpers at their + * limits (infinity, NaN, DBL_MAX, channel ids past the colours). + * + * SPDX-License-Identifier: MIT + * SPDX-FileCopyrightText: Copyright (c) 2026 Bartlomiej Mucha + */ + +#define MINIPSD_STATIC +#define MINIPSD_IMPLEMENTATION +#include "minipsd.h" + +#include "support.h" + +#include +#include +#include + +namespace { + +minipsd_resource resourceOf(std::uint16_t id, const Bytes &data) +{ + return minipsd_resource{id, {data.data(), data.size()}, {nullptr, 0}}; +} + +// --------------------------------------------------------------------------------------------------- +// the rows: encode, predict, matte +// --------------------------------------------------------------------------------------------------- + +TEST(MinipsdEdgeTest, RowFunctionsRefuseEachNullAndBadDepth) +{ + const std::uint8_t row[4] = {1, 2, 3, 4}; + std::uint8_t out[16]; + std::size_t size = 0; + + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 8, nullptr, 1, 4, out, sizeof out, &size), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 8, row, 1, 4, nullptr, sizeof out, &size), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 8, row, 1, 4, out, sizeof out, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_encode_row(99, 8, row, 1, 4, out, sizeof out, &size), MINIPSD_ERR_ARG) << "no such compression"; + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 7, row, 1, 4, out, sizeof out, &size), MINIPSD_ERR_ARG) << "no such depth"; + + // Too small by one byte: refused, and the size it needs reported. + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 8, row, 1, 4, out, 3, &size), MINIPSD_ERR_SIZE); + EXPECT_EQ(size, 4u); + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 8, row, 1, 4, out, 4, &size), MINIPSD_OK); + EXPECT_EQ(size, 4u); + // An empty row: nothing raw. + EXPECT_EQ(minipsd_encode_row(MINIPSD_RAW, 8, row, 1, 0, out, sizeof out, &size), MINIPSD_OK); + EXPECT_EQ(size, 0u); + + EXPECT_EQ(minipsd_predict_row(8, nullptr, 1, 4, out, sizeof out), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_predict_row(8, row, 1, 4, nullptr, sizeof out), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_predict_row(1, row, 1, 4, out, sizeof out), MINIPSD_ERR_ARG) << "1-bit has no prediction"; + EXPECT_EQ(minipsd_predict_row(7, row, 1, 4, out, sizeof out), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_predict_row(8, row, 1, 4, out, 3), MINIPSD_ERR_SIZE) << "one byte short"; + EXPECT_EQ(minipsd_predict_row(8, row, 1, 4, out, 4), MINIPSD_OK); + EXPECT_EQ(Bytes(out, out + 4), (Bytes{1, 1, 1, 1})) << "the differences"; + + std::uint8_t pixels[4] = {0, 0, 0, 0}; + EXPECT_EQ(minipsd_matte_row(MINIPSD_MODE_RGB, 8, nullptr, 1), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_matte_row(MINIPSD_MODE_RGB, 1, pixels, 1), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_matte_row(MINIPSD_MODE_RGB, 7, pixels, 1), MINIPSD_ERR_ARG); +} + +// --------------------------------------------------------------------------------------------------- +// the resource setters (builders.h) +// --------------------------------------------------------------------------------------------------- + +TEST(MinipsdEdgeTest, VersionInfoNamesWithoutDataAreRefused) +{ + minipsd_version_info vi{}; + std::size_t needed = 0; + // Both names empty, no data: valid, the head and two empty strings. + EXPECT_EQ(minipsd_resource_make_version_info(&vi, nullptr, 0, &needed), MINIPSD_MORE); + EXPECT_GT(needed, 0u); + + vi.writer_name = {nullptr, 2}; // a size without data + EXPECT_EQ(minipsd_resource_make_version_info(&vi, nullptr, 0, &needed), MINIPSD_ERR_ARG); + vi.writer_name = {nullptr, 0}; + vi.reader_name = {nullptr, 2}; + EXPECT_EQ(minipsd_resource_make_version_info(&vi, nullptr, 0, &needed), MINIPSD_ERR_ARG); + + const std::uint8_t unit[3] = {0, 'A', 0}; + vi.reader_name = {nullptr, 0}; + vi.writer_name = {unit, 3}; // half a UTF-16 unit, in the writer's name this time + EXPECT_EQ(minipsd_resource_make_version_info(&vi, nullptr, 0, &needed), MINIPSD_ERR_ARG); +} + +TEST(MinipsdEdgeTest, ResourceSettersTakeEmptyListsAndRefuseOverflowingCounts) +{ + const minipsd_grid_and_guides grid{1, 576, 576, 0}; + std::size_t needed = 0; + + // A count past what the file's four bytes hold: refused before a guide is read. + if constexpr (sizeof(std::size_t) > sizeof(std::uint32_t)) { + const minipsd_guide one{0, 0}; + const std::size_t tooMany = static_cast(std::numeric_limits::max()) + 1; + EXPECT_EQ(minipsd_resource_make_grid_and_guides(&grid, &one, tooMany, nullptr, 0, &needed), MINIPSD_ERR_ARG); + } + + // Alpha names: the Unicode id with a count and no names; no names at all is an empty list. + EXPECT_EQ(minipsd_resource_make_alpha_names(MINIPSD_RES_UNICODE_ALPHA_NAMES, nullptr, 1, nullptr, 0, &needed), + MINIPSD_ERR_ARG); + needed = 99; + EXPECT_EQ(minipsd_resource_make_alpha_names(MINIPSD_RES_UNICODE_ALPHA_NAMES, nullptr, 0, nullptr, 0, &needed), MINIPSD_OK); + EXPECT_EQ(needed, 0u); + needed = 99; + EXPECT_EQ(minipsd_resource_make_alpha_names(MINIPSD_RES_ALPHA_NAMES, nullptr, 0, nullptr, 0, &needed), MINIPSD_OK); + EXPECT_EQ(needed, 0u); + + // Alpha identifiers: none, with no array, is an empty list. + needed = 99; + EXPECT_EQ(minipsd_resource_make_alpha_identifiers(nullptr, 0, nullptr, 0, &needed), MINIPSD_OK); + EXPECT_EQ(needed, 0u); +} + +TEST(MinipsdEdgeTest, BlockSettingsTakeAnEmptyRunInEitherFormat) +{ + minipsd_block_settings set{}; + std::size_t needed = 0; + const minipsd_bytes empty{nullptr, 0}, noData{nullptr, 4}; + + // An empty run with no data: a fresh layer, in a PSD and in a PSB. + EXPECT_EQ(minipsd_layer_make_block_settings(&empty, MINIPSD_PSD, &set, nullptr, 0, &needed), MINIPSD_OK); + EXPECT_EQ(minipsd_layer_make_block_settings(&empty, MINIPSD_PSB, &set, nullptr, 0, &needed), MINIPSD_OK); + EXPECT_EQ(minipsd_layer_make_block_settings(nullptr, MINIPSD_PSB, &set, nullptr, 0, &needed), MINIPSD_OK); + + minipsd_block_settings back{}; + EXPECT_EQ(minipsd_blocks_settings(&empty, MINIPSD_PSB, &back), MINIPSD_OK); + EXPECT_EQ(back.present, 0u) << "nothing in an empty run"; + EXPECT_EQ(minipsd_blocks_settings(&empty, 0, &back), MINIPSD_OK) << "0: the PSD layout"; + EXPECT_EQ(minipsd_blocks_settings(&empty, MINIPSD_PSD, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_blocks_settings(&noData, MINIPSD_PSD, &back), MINIPSD_ERR_ARG) << "a size without data"; +} + +// --------------------------------------------------------------------------------------------------- +// the resource views (views.h) +// --------------------------------------------------------------------------------------------------- + +TEST(MinipsdEdgeTest, ResourceViewsRefuseEachNullOutput) +{ + const Bytes resolution(16, 0); + const minipsd_resource ri = resourceOf(MINIPSD_RES_RESOLUTION_INFO, resolution); + EXPECT_EQ(minipsd_resource_resolution_info(&ri, nullptr), MINIPSD_ERR_ARG); + + const Bytes header(16, 0); // a grid, no guides + const minipsd_resource g = resourceOf(MINIPSD_RES_GRID_AND_GUIDES, header); + std::size_t pos = 0; + minipsd_guide guide{}; + EXPECT_EQ(minipsd_resource_grid_and_guides(&g, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_resource_next_guide(&g, &pos, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_resource_next_guide(nullptr, &pos, &guide), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_resource_next_guide(&g, &pos, &guide), MINIPSD_OK) << "no guides: the end at once"; + + const Bytes ids(8, 0); + const minipsd_resource ai = resourceOf(MINIPSD_RES_ALPHA_IDENTIFIERS, ids); + std::uint32_t id = 0; + EXPECT_EQ(minipsd_resource_next_alpha_identifier(&ai, nullptr, &id), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_resource_next_alpha_identifier(nullptr, &pos, &id), MINIPSD_ERR_ARG); + + const Bytes pascal{1, 'A'}; + const minipsd_resource an = resourceOf(MINIPSD_RES_ALPHA_NAMES, pascal); + EXPECT_EQ(minipsd_resource_next_alpha_name(&an, &pos, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_resource_next_alpha_name(&an, nullptr, nullptr), MINIPSD_ERR_ARG); + const minipsd_resource noData{MINIPSD_RES_UNICODE_ALPHA_NAMES, {nullptr, 4}, {nullptr, 0}}; + minipsd_bytes name{}; + pos = 0; + EXPECT_EQ(minipsd_resource_next_alpha_name(&noData, &pos, &name), MINIPSD_ERR_ARG) << "a size without data"; +} + +TEST(MinipsdEdgeTest, ResourceViewsRefuseOverflowingCountsAndShortHeaders) +{ + minipsd_grid_and_guides grid{}; + minipsd_guide guide{}; + std::size_t pos = 0; + + // A header one byte short. + const Bytes shortHead(15, 0); + const minipsd_resource s = resourceOf(MINIPSD_RES_GRID_AND_GUIDES, shortHead); + EXPECT_EQ(minipsd_resource_grid_and_guides(&s, &grid), MINIPSD_ERR_FORMAT); + EXPECT_EQ(minipsd_resource_next_guide(&s, &pos, &guide), MINIPSD_ERR_FORMAT); + + // 0xFFFFFFFF guides of five bytes: the product does not fit 32 bits; refused, not wrapped. + const Bytes huge{0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0xFF, 0xFF}; + const minipsd_resource h = resourceOf(MINIPSD_RES_GRID_AND_GUIDES, huge); + grid.version = 77; + EXPECT_EQ(minipsd_resource_grid_and_guides(&h, &grid), MINIPSD_ERR_FORMAT); + EXPECT_EQ(grid.version, 77u) << "untouched on failure"; + + // 0xFFFFFFFF UTF-16 units: twice that overflows 32 bits; refused, not wrapped. + const Bytes units{0xFF, 0xFF, 0xFF, 0xFF, 0, 'A'}; + const minipsd_resource u = resourceOf(MINIPSD_RES_UNICODE_ALPHA_NAMES, units); + minipsd_bytes name{}; + pos = 0; + EXPECT_EQ(minipsd_resource_next_alpha_name(&u, &pos, &name), MINIPSD_ERR_FORMAT); +} + +TEST(MinipsdEdgeTest, CursorsOutsideAListAreRefusedOrEndIt) +{ + // A guide: the header, then one guide at x = 1. + const Bytes one{0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 32, 0}; + const minipsd_resource g = resourceOf(MINIPSD_RES_GRID_AND_GUIDES, one); + minipsd_guide guide{}; + std::size_t pos = 4; // inside the header: not where a guide starts + EXPECT_EQ(minipsd_resource_next_guide(&g, &pos, &guide), MINIPSD_ERR_ARG); + pos = 16 + 5 * 200; // aligned, past the last guide: the end + EXPECT_EQ(minipsd_resource_next_guide(&g, &pos, &guide), MINIPSD_OK); + + const Bytes ids(8, 0); + const minipsd_resource ai = resourceOf(MINIPSD_RES_ALPHA_IDENTIFIERS, ids); + std::uint32_t id = 0; + pos = 2; // inside an identifier + EXPECT_EQ(minipsd_resource_next_alpha_identifier(&ai, &pos, &id), MINIPSD_ERR_ARG); + pos = 8; // at the end + EXPECT_EQ(minipsd_resource_next_alpha_identifier(&ai, &pos, &id), MINIPSD_OK); + pos = 400; // past the end + EXPECT_EQ(minipsd_resource_next_alpha_identifier(&ai, &pos, &id), MINIPSD_OK); + + const Bytes pascal{1, 'A'}; + const minipsd_resource an = resourceOf(MINIPSD_RES_ALPHA_NAMES, pascal); + minipsd_bytes name{}; + pos = 400; + EXPECT_EQ(minipsd_resource_next_alpha_name(&an, &pos, &name), MINIPSD_OK); +} + +// --------------------------------------------------------------------------------------------------- +// the reader (reader.h) +// --------------------------------------------------------------------------------------------------- + +TEST(MinipsdEdgeTest, DecodeMaskRefusesANullLayerAndAReaderNotReady) +{ + minipsd_reader r; + std::memset(&r, 0, sizeof r); // never initialised: not ready + minipsd_layer_view layer{}; + layer.has_mask = 1; + std::uint8_t sample = 0; + minipsd_plane plane{&sample, 1, 1}; + + EXPECT_EQ(minipsd_decode_mask(&r, nullptr, &plane, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_decode_mask(&r, &layer, nullptr, nullptr), MINIPSD_ERR_ARG); + EXPECT_EQ(minipsd_decode_mask(&r, &layer, &plane, nullptr), MINIPSD_ERR_ARG) << "not ready"; +} + +TEST(MinipsdEdgeTest, ChannelIdsMapToSlotsAtTheLimits) +{ + minipsd_reader r; + std::memset(&r, 0, sizeof r); + + r.colors = 3; // RGB: three colours, then transparency + EXPECT_EQ(mpsd__planes(&r), 4); + EXPECT_EQ(mpsd__slot(&r, 0), 0); + EXPECT_EQ(mpsd__slot(&r, 2), 2); + EXPECT_EQ(mpsd__slot(&r, 3), -1) << "a spot channel: within the planes, past the colours"; + EXPECT_EQ(mpsd__slot(&r, 9), -1); + EXPECT_EQ(mpsd__slot(&r, -1), 3) << "transparency after the colours"; + EXPECT_EQ(mpsd__slot(&r, -2), MPSD__SLOT_MASK); + EXPECT_EQ(mpsd__slot(&r, -3), MPSD__SLOT_REAL); + EXPECT_EQ(mpsd__slot(&r, -4), -1) << "no such id"; + EXPECT_EQ(mpsd__slot(&r, std::numeric_limits::min()), -1); + + r.colors = MINIPSD_PLANES; // as many colours as planes: no room for transparency + EXPECT_EQ(mpsd__planes(&r), MINIPSD_PLANES); + EXPECT_EQ(mpsd__slot(&r, -1), -1); + EXPECT_EQ(mpsd__slot(&r, MINIPSD_PLANES - 1), MINIPSD_PLANES - 1); + EXPECT_EQ(mpsd__slot(&r, MINIPSD_PLANES), -1); +} + +// --------------------------------------------------------------------------------------------------- +// the internal helpers at their limits +// --------------------------------------------------------------------------------------------------- + +TEST(MinipsdEdgeTest, FloatHelpersAtInfinityNanAndTheExtremes) +{ + const double inf = std::numeric_limits::infinity(); + const double nan = std::numeric_limits::quiet_NaN(); + + EXPECT_TRUE(mpsd__floats_equal(1.0, 1.0 + 1e-12)); + EXPECT_FALSE(mpsd__floats_equal(1.0, 2.0)) << "b above a"; + EXPECT_FALSE(mpsd__floats_equal(2.0, 1.0)) << "a above b"; + EXPECT_FALSE(mpsd__floats_equal(nan, nan)); + EXPECT_FALSE(mpsd__floats_equal(inf, inf)) << "inf - inf is NaN"; + + EXPECT_TRUE(mpsd__floats_positive(DBL_MIN)); + EXPECT_TRUE(mpsd__floats_positive(DBL_MAX)); + EXPECT_FALSE(mpsd__floats_positive(0.0)); + EXPECT_FALSE(mpsd__floats_positive(-0.0)); + EXPECT_FALSE(mpsd__floats_positive(-1.0)); + EXPECT_FALSE(mpsd__floats_positive(inf)) << "past DBL_MAX"; + EXPECT_FALSE(mpsd__floats_positive(-inf)); + EXPECT_FALSE(mpsd__floats_positive(nan)); +} + +TEST(MinipsdEdgeTest, ZipScratchGrowsOnlyForZipChannelsWithData) +{ + mpsd__arena a{}; + const std::uint8_t zip[2] = {0, MPSD__COMP_ZIP}, rle[2] = {0, 1}; + + mpsd__zip_room(&a, minipsd_bytes{nullptr, 2}, 100); + EXPECT_EQ(a.zip, 0u) << "a size without data"; + mpsd__zip_room(&a, minipsd_bytes{zip, 1}, 100); + EXPECT_EQ(a.zip, 0u) << "shorter than the marker"; + mpsd__zip_room(&a, minipsd_bytes{rle, 2}, 100); + EXPECT_EQ(a.zip, 0u) << "not ZIP"; + mpsd__zip_room(&a, minipsd_bytes{zip, 2}, 100); + EXPECT_EQ(a.zip, 100u); + mpsd__zip_room(&a, minipsd_bytes{zip, 2}, 50); + EXPECT_EQ(a.zip, 100u) << "the largest kept"; + mpsd__zip_room(&a, minipsd_bytes{zip, 2}, std::numeric_limits::max()); + EXPECT_EQ(a.zip, std::numeric_limits::max()); +} + +} // namespace From c7becf69acd84b66a15f0ccabb4628038db56108 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 23:04:04 +0000 Subject: [PATCH 2/4] test(crafted): crafted files and documents for the reader's and writer's untaken branches Files built with the reader tests' PSD builder (malformed ones are made in code, as the others are) and documents with crafted kept bytes: - reader: pixel aspect ratios that are not positive (0, -1, inf, NaN) and a duplicate 1064; bitmap at 8 bits and greyscale at 1 bit; a composite alpha without its channel; a stored composite short of colour channels; layers and masks without height or width; a mask that does not decode, and the same asked for no rows; channel data past the layer info; a PSB layer taller than any format allows; multichannel filling every plane; a fully transparent 32-bit pixel un-matted; 'Mtrn'/'Mt16' and a short tail - writer: a kept resource name without data, an inverted mask, 32-bit greyscale, multichannel given pixels or no channel count, a kept tail ending in stray bytes (8BIM, 8B64, a cut PSB long-key header) kept or converted, and a kept 'lsct' too short for its type Branches 96.9% -> 98.2% (read.h 92% -> 97%, reader.h 97% -> 99%, writer.h 95% -> 96%); clean under clang-tidy and AddressSanitizer. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019egXEpyVAm2D3tvQn2Xcuv --- tests/test_reader.cpp | 309 ++++++++++++++++++++++++++++++++++++++++++ tests/test_writer.cpp | 115 ++++++++++++++++ 2 files changed, 424 insertions(+) diff --git a/tests/test_reader.cpp b/tests/test_reader.cpp index 86ac883..d92913f 100644 --- a/tests/test_reader.cpp +++ b/tests/test_reader.cpp @@ -18,11 +18,13 @@ #include "zlib_stored.h" #include +#include #include #include #include #include #include +#include #include #include #include @@ -3509,3 +3511,310 @@ TEST(MinipsdReaderTest, FilesFromOtherProgramsAreReadWholeAndInParts) EXPECT_LE(have, file.size()); } } + +// --------------------------------------------------------------------------------------------------- +// Crafted files for the branches the files above leave untaken: a value the reader must refuse or +// ignore, a zero-area layer or mask, a mode at the edge of the planes, a channel that does not +// decode. +// --------------------------------------------------------------------------------------------------- + +namespace { + +Bytes aspectRatio(double ratio) // resource 1064: its version, then the ratio as a big-endian double +{ + std::uint64_t bits = 0; + std::memcpy(&bits, &ratio, sizeof bits); + return Out().u32(MPSD__PIXEL_ASPECT_VERSION).u64(bits).b; +} + +Spec flatRgb() // 2 x 2 RGB, no layers, a raw composite +{ + Spec s; + s.channels = 3; + s.count = 0; + s.image = rawImage(3, 2, 2, 0x80); + return s; +} + +Bytes maskRect(int top, int left, int bottom, int right) // a layer mask record: the rectangle, colour, flags +{ + return Out().u32(static_cast(top)).u32(static_cast(left)) + .u32(static_cast(bottom)).u32(static_cast(right)).u8(0).u8(0).zeros(2).b; +} + +} // namespace + +// A pixel aspect ratio the writer would not take (zero, negative, infinite, NaN) is none: the +// document gets 0; a second 1064 is ignored, the first one counts. Read under a decoding limit +// it stays within. +TEST(MinipsdCraftedTest, APixelAspectRatioThatIsNotPositiveIsDropped) +{ + const double inf = std::numeric_limits::infinity(); + const struct { double ratio, kept; } cases[] = { + {2.0, 2.0}, {0.0, 0.0}, {-1.0, 0.0}, {inf, 0.0}, {std::numeric_limits::quiet_NaN(), 0.0}, + }; + minipsd_limits lim{}; + lim.max_decoded_bytes = 1u << 20; + for (const auto &c : cases) { + SCOPED_TRACE(c.ratio); + Spec s = flatRgb(); + s.resources = Out().raw(resource(1005, resolution(72))).raw(resource(1064, aspectRatio(c.ratio))) + .raw(resource(1064, aspectRatio(3.0))).b; // the second: ignored + const Bytes file = build(s).bytes; + Open o(file); + ASSERT_EQ(o.rc, MINIPSD_OK); + if (!std::isnan(c.ratio)) { + EXPECT_EQ(o.info.pixel_aspect_ratio, c.ratio) << "the first 1064"; + } + const Doc d = readDoc(file, nullptr, MINIPSD_OK, &lim); + ASSERT_TRUE(d); + EXPECT_EQ(d->pixel_aspect_ratio, c.kept); + } +} + +// The bitmap mode is 1-bit and only it: a bitmap of 8 bits, or a 1-bit greyscale, is refused. +TEST(MinipsdCraftedTest, ABitmapModeAndA1BitDepthGoTogether) +{ + for (const auto &[mode, depth] : {std::pair{MINIPSD_MODE_BITMAP, 8}, {MINIPSD_MODE_GREY, 1}}) { + SCOPED_TRACE(mode); + Spec s = flatRgb(); + s.mode = mode; + s.depth = depth; + s.channels = 1; + s.image = rawImage(1, 2, 2, 0); + readDoc(build(s).bytes, nullptr, MINIPSD_ERR_UNSUPPORTED); + } +} + +// A negative layer count says the composite has transparency, but three channels leave no room +// for it: the composite is refused, not read past its channels. +TEST(MinipsdCraftedTest, ACompositeAlphaWithoutItsChannelIsRefused) +{ + Spec s; + s.channels = 3; + s.layers = {simpleRec()}; + s.count = -1; + s.image = rawImage(3, 2, 2, 0x80); + const Bytes file = build(s).bytes; + Open o(file); + ASSERT_EQ(o.rc, MINIPSD_OK); + EXPECT_NE(o.info.composite_has_alpha, 0); + readDoc(file, nullptr, MINIPSD_ERR_FORMAT); +} + +// Stored, a composite with fewer channels than the mode's colours cannot be kept as it is: it is +// decoded instead, and refused for the channels it lacks. +TEST(MinipsdCraftedTest, AStoredCompositeShortOfColourChannelsIsDecoded) +{ + Spec s = flatRgb(); + s.channels = 1; + s.image = rawImage(1, 2, 2, 0x80); + const minipsd_read_opts stored{1, nullptr, nullptr}; + readDoc(build(s).bytes, nullptr, MINIPSD_ERR_FORMAT, nullptr, &stored); +} + +// A layer and a mask with a width but no height: nothing to decode, nothing to keep, in either +// way of reading. +TEST(MinipsdCraftedTest, ALayerAndAMaskWithoutHeightHaveNoPixels) +{ + Rec flat; + flat.right = 2; // bottom 0: no rows + flat.channels = {chan(-1, 2, 0), chan(0, 2, 0), chan(1, 2, 0), chan(2, 2, 0)}; + flat.pascal = "F"; + Rec masked = simpleRec(); + masked.mask = maskRect(0, 0, 0, 2); // the mask: no rows either, and no channel for it + Spec s; + s.layers = {flat, masked}; + s.count = -2; + s.image = rawImage(4, 2, 2, 0x80); + const Bytes file = build(s).bytes; + for (const int stored : {0, 1}) { + SCOPED_TRACE(stored); + const minipsd_read_opts opts{stored, nullptr, nullptr}; + const Doc d = readDoc(file, nullptr, MINIPSD_OK, nullptr, &opts); + ASSERT_TRUE(d); + ASSERT_EQ(d->layer_count, 2u); + EXPECT_EQ(d->layers[0].pixels, nullptr); + EXPECT_EQ(d->layers[0].stored, nullptr); + ASSERT_NE(d->layers[1].mask, nullptr); + EXPECT_EQ(d->layers[1].mask->pixels, nullptr); + EXPECT_EQ(d->layers[1].mask->stored.size, 0u); + } +} + +// A mask whose channel does not decode fails the whole read. +TEST(MinipsdCraftedTest, AMaskThatDoesNotDecodeFailsTheRead) +{ + Rec r = simpleRec(); + r.mask = maskRect(0, 0, 2, 2); + r.channels.push_back(Chan{-2, Out().u16(1).u16(1).u16(1).u8(0x80).u8(0x80).b}); // RLE: a no-op where a row is + Spec s; + s.layers = {r}; + s.count = -1; + s.image = rawImage(4, 2, 2, 0x80); + const Bytes file = build(s).bytes; + readDoc(file, nullptr, MINIPSD_ERR_FORMAT); + + // Asked for no rows (from the last), the same mask decodes to nothing, its data never read. + Open o(file); + ASSERT_EQ(o.rc, MINIPSD_OK); + const minipsd_layer_view v = nextLayer(o.r); + std::uint8_t px[4]; + const minipsd_plane plane{px, 1, 2}; + const minipsd_decode_opts fromEnd{2, 0, 0, 8}; + EXPECT_EQ(minipsd_decode_mask(o.r, &v, &plane, &fromEnd), MINIPSD_OK); +} + +// read_into: a file that does not measure is refused before the block is touched. +TEST(MinipsdCraftedTest, ReadIntoRefusesAFileItCannotMeasure) +{ + const Bytes garbage(40, 'x'); + Bytes block(64, 0xEE); + minipsd_doc *d = reinterpret_cast(1); + EXPECT_EQ(minipsd_read_into(garbage.data(), garbage.size(), nullptr, nullptr, block.data(), block.size(), &d, nullptr), + MINIPSD_ERR_FORMAT); + EXPECT_EQ(d, nullptr); + EXPECT_EQ(block, Bytes(64, 0xEE)); +} + +// An interleaved target of more planes than there are takes the most there are; a depth of 0 +// is 8 bits. +TEST(MinipsdCraftedTest, AnInterleavedTargetClampsItsPlanes) +{ + std::uint8_t px[2 * MINIPSD_PLANES] = {}; + minipsd_target t; + minipsd_target_interleaved(&t, px, sizeof px, 9, 0); + for (int k = 0; k < MINIPSD_PLANES; ++k) { + EXPECT_EQ(t.channel[k].data, px + k) << k; + EXPECT_EQ(t.channel[k].pixel_step, MINIPSD_PLANES) << k; + } +} + +// At 32 bits, a fully transparent pixel has no colour to un-matte: it is 0, not a division by 0. +TEST(MinipsdCraftedTest, AFullyTransparentFloatPixelUnmattesToZero) +{ + const std::vector px = {0.25f, 0.5f, 0.75f, 0.0f, 0.25f, 0.5f, 0.75f, 1.0f}; // 2 x 1: clear, opaque + const minipsd_layer layer{.name = "L", .pixels = reinterpret_cast(px.data()), .opacity = 255}; + minipsd_doc doc = docOf(2, 1, &layer, 1); + doc.depth = 32; + doc.composite = layer.pixels; + Open o(write(doc)); + ASSERT_EQ(o.rc, MINIPSD_OK); + float back[8]; + minipsd_target t; + const minipsd_decode_opts opts{0, 0, 0, 32}; // un-matted + minipsd_target_interleaved(&t, back, sizeof back, 4, 32); + ASSERT_EQ(minipsd_decode_composite(o.r, &t, &opts), MINIPSD_OK); + EXPECT_EQ(back[0], 0.0f) << "clear: no colour"; + EXPECT_EQ(back[3], 0.0f); + EXPECT_FLOAT_EQ(back[4], 0.25f) << "opaque: as given"; +} + +// Multichannel with as many channels as there are planes: every plane a colour, none left for +// transparency; the composite decodes plane by plane. +TEST(MinipsdCraftedTest, AMultichannelCompositeFillsEveryPlane) +{ + Spec s = flatRgb(); + s.mode = MINIPSD_MODE_MULTICHANNEL; + s.channels = MINIPSD_PLANES; + s.image = rawImage(MINIPSD_PLANES, 2, 2, 0x40); + Open o(build(s).bytes); + ASSERT_EQ(o.rc, MINIPSD_OK); + Bytes px(std::size_t{2} * 2 * MINIPSD_PLANES, 0); + minipsd_target t; + minipsd_target_interleaved(&t, px.data(), std::size_t{2} * MINIPSD_PLANES, MINIPSD_PLANES, 8); + ASSERT_EQ(minipsd_decode_composite(o.r, &t, nullptr), MINIPSD_OK); + EXPECT_EQ(px, Bytes(px.size(), 0x40)); +} + +// A PSB layer taller than any format allows is refused when decoded: its plane's size could wrap. +TEST(MinipsdCraftedTest, ALayerTallerThanAPsbAllowsIsNotDecoded) +{ + Rec r; + r.right = 1; + r.bottom = MINIPSD_MAX_SIDE_PSB + 1; + r.channels = {chan(-1, 2, 0), chan(0, 2, 0), chan(1, 2, 0), chan(2, 2, 0)}; + Spec s; + s.version = 2; + s.layers = {r}; + s.count = -1; + s.image = rawImage(4, 2, 2, 0x80); + Open o(build(s).bytes); + ASSERT_EQ(o.rc, MINIPSD_OK); + const minipsd_layer_view v = nextLayer(o.r); + Bytes px(4); + EXPECT_EQ(decodeRgba(o.r, v, 1, px, 0, 1), MINIPSD_ERR_UNSUPPORTED); +} + +// The channel data of the layers, added up, longer than the layer info that holds it: refused by +// the first next_layer, which walks the records to find where the data starts. +TEST(MinipsdCraftedTest, ChannelDataPastTheLayerInfoIsRefused) +{ + Rec r = simpleRec(); + r.channels.back().lengthDelta = 4; // declares four bytes the section does not have + Spec s; + s.layers = {r}; + s.count = -1; + s.image = rawImage(4, 2, 2, 0x80); + const Bytes file = build(s).bytes; + Open o(file); + ASSERT_EQ(o.rc, MINIPSD_OK); + nextLayer(o.r, MINIPSD_ERR_FORMAT); + readDoc(file, nullptr, MINIPSD_ERR_FORMAT); // and the whole read with it, at its first layer +} + +// A mask with rows but no columns: nothing to decode, not even a plane to write to. +TEST(MinipsdCraftedTest, AMaskWithoutWidthDecodesToNothing) +{ + Rec r = simpleRec(); + r.mask = maskRect(0, 0, 2, 0); + r.channels.push_back(chan(-2, 2, 0)); + Spec s; + s.layers = {r}; + s.count = -1; + s.image = rawImage(4, 2, 2, 0x80); + Open o(build(s).bytes); + ASSERT_EQ(o.rc, MINIPSD_OK); + const minipsd_layer_view v = nextLayer(o.r); + ASSERT_NE(v.has_mask, 0); + const minipsd_plane none{nullptr, 1, 0}; + EXPECT_EQ(minipsd_decode_mask(o.r, &v, &none, nullptr), MINIPSD_OK); +} + +// Multichannel read with options that do not ask for stored channels: kept as stored all the same, +// as multichannel always is. +TEST(MinipsdCraftedTest, MultichannelIsKeptStoredWithOrWithoutTheOption) +{ + Spec s = flatRgb(); + s.mode = MINIPSD_MODE_MULTICHANNEL; + s.channels = 2; + s.image = rawImage(2, 2, 2, 0x40); + const Bytes file = build(s).bytes; + for (const int stored : {0, 1}) { + SCOPED_TRACE(stored); + const minipsd_read_opts opts{stored, nullptr, nullptr}; + const Doc d = readDoc(file, nullptr, MINIPSD_OK, nullptr, &opts); + ASSERT_TRUE(d); + EXPECT_EQ(d->composite, nullptr); + EXPECT_EQ(d->composite_channels, 2); + } +} + +// Global blocks: 'Mtrn' and 'Mt16' each say the composite's first extra channel is transparency; +// a few bytes after the last block, too short for a header, end the walk quietly. +TEST(MinipsdCraftedTest, TransparencyBlocksAndAShortTailAfterThem) +{ + const Bytes mask(4, 0); + for (const char *key : {"Mtrn", "Mt16"}) { + SCOPED_TRACE(key); + Spec s; + s.hasRawSection = true; + const Bytes tail = Out().raw(mask).str("8BIM").str(key).u32(0).zeros(4).b; + s.rawLayerSection = Out().u32(static_cast(4 + tail.size())).u32(0).raw(tail).b; + s.image = rawImage(4, 2, 2, 0x80); + Open o(build(s).bytes); + ASSERT_EQ(o.rc, MINIPSD_OK); + EXPECT_NE(o.info.composite_has_alpha, 0); + EXPECT_EQ(o.info.layer_count, 0u); + } +} diff --git a/tests/test_writer.cpp b/tests/test_writer.cpp index b7ecc1a..05dc65d 100644 --- a/tests/test_writer.cpp +++ b/tests/test_writer.cpp @@ -1493,3 +1493,118 @@ TEST(MinipsdWriterTest, CompiledAsCItWritesTheSameBytes) EXPECT_EQ(Bytes(buf, buf + size), write(docOf(3, 2, layers, 2))); } + +// --------------------------------------------------------------------------------------------------- +// Crafted documents and kept bytes for the branches the tests above leave untaken. +// --------------------------------------------------------------------------------------------------- + +// A kept resource's name: a size without data is the caller's mistake. +TEST(MinipsdCraftedWriterTest, AKeptResourceNameWithoutDataIsRefused) +{ + const Bytes px = testImage(2, 2), data = {1, 2}; + const minipsd_layer layer{"L", px.data(), 255, 0}; + const minipsd_resource kept{1000, view(data), {nullptr, 3}}; + minipsd_doc d = docOf(2, 2, &layer, 1); + d.resources = &kept; + d.resource_count = 1; + EXPECT_EQ(minipsd_doc_check(&d, nullptr), MINIPSD_ERR_ARG); +} + +// A mask whose right edge is left of its left edge: inverted, refused. +TEST(MinipsdCraftedWriterTest, AnInvertedMaskIsRefused) +{ + const Bytes px = testImage(2, 2), ramp(4, 0x80); + const minipsd_mask mask{.top = 0, .left = 2, .bottom = 2, .right = 0, .pixels = ramp.data()}; + const minipsd_layer layer{.name = "L", .pixels = px.data(), .opacity = 255, .mask = &mask}; + const minipsd_doc d = docOf(2, 2, &layer, 1); + EXPECT_EQ(minipsd_doc_check(&d, nullptr), MINIPSD_ERR_ARG); +} + +// 32 bits: greyscale is taken as RGB is; multichannel's composite is only ever copied, never given +// as pixels. +TEST(MinipsdCraftedWriterTest, ModesAtTheEdgeOfWhatTheWriterTakes) +{ + const std::vector grey = {0.0f, 0.25f, 0.5f, 1.0f}; // 2 x 2, one channel + minipsd_doc g{}; + g.width = 2; g.height = 2; g.dpi = 72.0; + g.depth = 32; + g.color_mode = MINIPSD_MODE_GREY; + g.composite = reinterpret_cast(grey.data()); + EXPECT_EQ(minipsd_doc_check(&g, nullptr), MINIPSD_OK); + const Bytes file = write(g); + EXPECT_FALSE(file.empty()); + + const Bytes planes(std::size_t{2} * 2 * 2, 0x40); + minipsd_doc m{}; + m.width = 2; m.height = 2; m.dpi = 72.0; + m.color_mode = MINIPSD_MODE_MULTICHANNEL; + m.composite_channels = 2; + m.composite = planes.data(); // pixels, not stored channels + EXPECT_EQ(minipsd_doc_check(&m, nullptr), MINIPSD_ERR_ARG); + m.composite = nullptr; + m.composite_stored = view(planes); + m.composite_channels = 0; // stored, but no count of its channels + EXPECT_EQ(minipsd_doc_check(&m, nullptr), MINIPSD_ERR_ARG); +} + +// A kept layer tail ending in a few bytes that are not a block: written as they are in the +// document's own format, refused when it must change format (its blocks re-written). +TEST(MinipsdCraftedWriterTest, AKeptTailEndingInStrayBytes) +{ + const Bytes px = testImage(2, 2); + const minipsd_layer layer{"L", px.data(), 255, 0}; + const Bytes tail = Bytes(4, 0) + tagged("Patt", {}) + Bytes(4, 'x'); + minipsd_doc d = docOf(2, 2, &layer, 1); + d.layer_tail = view(tail); + const Bytes same = write(d); + EXPECT_NE(std::search(same.begin(), same.end(), tail.begin(), tail.end()), same.end()) << "as they are"; + minipsd_write_opts psb{}; + psb.format = MINIPSD_PSB; + write(d, MINIPSD_ERR_UNSUPPORTED, &psb); + + // At 32 bits the writer looks in the tail for a layers' block: the stray bytes end the search. + const std::vector deep(std::size_t{2} * 2 * 4, 1.0f); + const minipsd_layer floats{"F", reinterpret_cast(deep.data()), 255, 0}; + minipsd_doc f = docOf(2, 2, &floats, 1); + f.depth = 32; + f.composite = floats.pixels; + f.layer_tail = view(tail); + const Bytes file = write(f); + EXPECT_NE(std::search(file.begin(), file.end(), tail.begin(), tail.end()), file.end()); + + // The same behind an '8B64' block, the other signature a block may have. + Bytes b64 = tagged("Patt", {}); + std::copy_n("8B64", 4, b64.begin()); + const Bytes tail64 = Bytes(4, 0) + b64 + Bytes(4, 'x'); + d.layer_tail = f.layer_tail = view(tail64); + write(d, MINIPSD_ERR_UNSUPPORTED, &psb); + EXPECT_FALSE(write(f).empty()); + + // In a PSB, a long key's header takes 16 bytes ('Alph': an eight-byte length); 12 left are + // not a header but stray bytes too. + Bytes alph = tagged("Alph", {}); + alph.resize(12); + const Bytes cut = Bytes(4, 0) + alph; + d.format = f.format = MINIPSD_PSB; + d.layer_tail = f.layer_tail = view(cut); + minipsd_write_opts psd{}; + psd.format = MINIPSD_PSD; + write(d, MINIPSD_ERR_UNSUPPORTED, &psd); + EXPECT_FALSE(write(f).empty()); +} + +// A kept 'lsct' too short to hold a section type says nothing: the group gets the writer's own. +TEST(MinipsdCraftedWriterTest, AKeptSectionBlockTooShortForItsTypeIsNotTheGroups) +{ + const Bytes shortLsct = tagged("lsct", {0, 0}); + const minipsd_layer layers[2] = { + {.name = "", .opacity = 255, .section_divider_type = MINIPSD_SECTION_GROUP_END}, + {.name = "Group", .opacity = 255, .additional_info = view(shortLsct), .section_divider_type = MINIPSD_SECTION_GROUP}}; + const minipsd_doc d = docOf(2, 2, layers, 2); + const Bytes file = write(d); + minipsd_doc *back = nullptr; + ASSERT_EQ(minipsd_read(file.data(), file.size(), nullptr, nullptr, &back, nullptr), MINIPSD_OK); + ASSERT_EQ(back->layer_count, 2u); + EXPECT_EQ(back->layers[1].section_divider_type, MINIPSD_SECTION_GROUP); + minipsd_doc_free(back); +} From 20e3625a28a6384b602f1b4226c65ddd6a9f65cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Mucha?= Date: Wed, 4 Nov 2026 00:42:15 +0100 Subject: [PATCH 3/4] fix(read): state the 8-byte alignment minipsd_read needs; findings from the test report - writer: mpsd__alpha_uncounted no longer repeats the flat-mode check composite_alpha makes - read: minipsd_read's alignment check is reachable with a MINIPSD_MALLOC that aligns less than malloc; documented (header, SPECIFICATION, MANUAL, CHANGELOG) and tested in test_alloc_failure.cpp - reader: comments on the window's base and on next_layer's guard; init's doc says later data is checked as it is read - tests: GoogleTest pinned by hash; targetOf takes 32 bits; offline note in CHEATSHEET - ci: branch coverage threshold 95 -> 97 % (98.3 % measured) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 4 ++-- docs/CHANGELOG.md | 6 ++++++ docs/CHEATSHEET.md | 6 ++++-- docs/CODE-ARCH.md | 2 +- docs/MANUAL.md | 2 +- docs/SPECIFICATION.md | 4 ++-- include/minipsd.h | 13 +++++++++---- include/minipsd/read.h | 2 +- include/minipsd/reader.h | 2 +- include/minipsd/writer.h | 2 +- tests/CMakeLists.txt | 1 + tests/test_alloc_failure.cpp | 27 ++++++++++++++++++++++----- tests/test_reader.cpp | 5 +++-- 13 files changed, 54 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99d8388..5ce4846 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,7 +114,7 @@ jobs: # No round trip: Python would need the sanitizer runtime preloaded. # linux-tsan ThreadSanitizer: decode calls run at once on one reader. # linux-tidy clang-tidy with .clang-tidy, findings are errors. - # linux-coverage coverage of minipsd.h, round trip included: lines 100 %, branches >= 95 %. + # linux-coverage coverage of minipsd.h, round trip included: lines 100 %, branches >= 97 %. analysis: if: ${{ !github.event.pull_request.draft }} name: ${{ matrix.preset }} @@ -155,7 +155,7 @@ jobs: run: | find build/linux-coverage \( -name '*.c.gcda' -o -name '*.c.gcno' \) -delete gcovr --root . --filter include/ --merge-lines --exclude-throw-branches \ - --fail-under-line 100 --fail-under-branch 95 \ + --fail-under-line 100 --fail-under-branch 97 \ --cobertura coverage.xml --txt coverage.txt --txt-metric branch --txt-summary \ build/linux-coverage || status=$? { echo '```'; cat coverage.txt; echo '```'; } >> "$GITHUB_STEP_SUMMARY" diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index e7d0d90..4c6aee8 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -3,6 +3,12 @@ Changes between releases, for users of minipsd. Versioning follows the pre-1.0 rules in [TODO.md](TODO.md): breaking changes bump the MINOR, additions and fixes the PATCH. +## [Unreleased] + +### Changed +- Stated what `minipsd_read` always required: a `MINIPSD_MALLOC` of your own must give blocks + aligned to 8 bytes, as `malloc`'s are, or `minipsd_read` returns `MINIPSD_ERR_ALLOC`. + ## [0.4.1] — 2026-11-03 ### Added diff --git a/docs/CHEATSHEET.md b/docs/CHEATSHEET.md index f0f5fbf..9861a95 100644 --- a/docs/CHEATSHEET.md +++ b/docs/CHEATSHEET.md @@ -53,7 +53,7 @@ The C units (the C smoke test, the examples, the fuzz replay) are left out, as C out: they check the header builds and behaves as C, and gcc numbers a line's branches differently in C, so gcovr would list those lines twice and the duplicates would hide which test is missing. Coverage data adds up across runs: delete `build/linux-coverage` for a clean report. -CI fails below 100 % of lines or 95 % of branches and publishes the report to Coveralls. +CI fails below 100 % of lines or 97 % of branches and publishes the report to Coveralls. ## Fuzzing @@ -160,7 +160,9 @@ paths also check the SBOM fragment's version and that it lists the headers. | `MINIPSD_TEST_FIXTURES` | ON (`linux-coverage`: OFF) | read the local PSDs in `tests/fixtures/` too; off, a local coverage run measures what CI's does | | `MINIPSD_CLANG_TIDY` | empty (`linux-tidy`: `clang-tidy`) | run clang-tidy on the test targets | -GoogleTest comes from the system if installed (`libgtest-dev`), otherwise it is fetched. +GoogleTest comes from the system if installed (`libgtest-dev`), otherwise it is fetched (v1.14.0, +pinned by hash). Offline, `-DFETCHCONTENT_SOURCE_DIR_GTEST=` takes a copy +of your own. The round trip needs Python with `psd-tools`; without it the test is skipped. A venv keeps it out of the system Python: diff --git a/docs/CODE-ARCH.md b/docs/CODE-ARCH.md index 40afdab..2c59a68 100644 --- a/docs/CODE-ARCH.md +++ b/docs/CODE-ARCH.md @@ -763,7 +763,7 @@ Every stage follows these rules: - **Independent reader for every byte change.** `psd-tools` in CI checks new bytes, as [Verification](SPECIFICATION.md#verification) requires. minipsd's own reader is never the only check of its own writer. -- **Coverage stays at 100 % of lines** and at least 95 % of branches, per module. A new module +- **Coverage stays at 100 % of lines** and at least 97 % of branches in all (95 % per module). A new module comes with its `test_.cpp`. - **The spec follows the code.** When a stage lands, its part of this file moves into [SPECIFICATION.md](SPECIFICATION.md), and this file keeps only what is still ahead. diff --git a/docs/MANUAL.md b/docs/MANUAL.md index 8e0559d..d7d070b 100644 --- a/docs/MANUAL.md +++ b/docs/MANUAL.md @@ -991,7 +991,7 @@ Define before the `#include` in the implementation file: | Macro | Effect | |---|---| | `MINIPSD_STATIC` | every function `static`, private to that file: several copies never clash, and those you do not call do not warn | -| `MINIPSD_MALLOC` / `MINIPSD_FREE` | replace `malloc` / `free`; define both | +| `MINIPSD_MALLOC` / `MINIPSD_FREE` | replace `malloc` / `free`; define both. `minipsd_read` needs blocks aligned to 8 bytes, as `malloc`'s are, or gives `MINIPSD_ERR_ALLOC` | | `MINIPSD_NO_MALLOC` | no allocation at all, and no ``: `minipsd_write` goes, the pull writer and the reader stay | | `MINIPSD_NO_READER` | no reader, typed views or `minipsd_read`: less code for a program that only writes | | `MINIPSD_NO_WRITER` | no writer, typed setters or `minipsd_read`: less code for a program that only reads | diff --git a/docs/SPECIFICATION.md b/docs/SPECIFICATION.md index 609c81b..ee7da5a 100644 --- a/docs/SPECIFICATION.md +++ b/docs/SPECIFICATION.md @@ -235,7 +235,7 @@ its layers and composite as stored: no colour is converted. | `MINIPSD_ERR_ARG` | -1 | NULL document or callback, no layers and no composite, ICC size without data, or a dpi that 16.16 fixed point cannot hold (see below) | | `MINIPSD_ERR_SIZE` | -2 | a side is 0 or above the format's (`MINIPSD_MAX_SIDE`, `MINIPSD_MAX_SIDE_PSB`), or a section is longer than its length field allows (see below) | | `MINIPSD_ERR_WRITE` | -3 | the callback returned 0 | -| `MINIPSD_ERR_ALLOC` | -4 | `MINIPSD_MALLOC` returned NULL | +| `MINIPSD_ERR_ALLOC` | -4 | `MINIPSD_MALLOC` returned NULL, or for `minipsd_read` a block aligned to less than 8 bytes | | `MINIPSD_ERR_OPEN` | -5 | not returned by minipsd; reserved for the caller's file code (`examples/write_file.c`) | | `MINIPSD_ERR_FORMAT` | -6 | reader: not a well-formed PSD | | `MINIPSD_ERR_UNSUPPORTED` | -7 | reader: a valid file outside what minipsd reads: for decoding, a file of another depth than `opts->depth`, or a ZIP-compressed channel without `opts->inflate`; for `minipsd_read`, a file the writer could not take back (a mode at a depth it does not write, such as 32-bit CMYK, or layers in a flat mode); writer: a mode at a depth it does not write; on a 32-bit platform, a PSB whose layer and mask section `size_t` cannot hold | @@ -886,7 +886,7 @@ Platemaker is the first consumer. Breaking any of these breaks it: with the correct dpi. - The same tests also run instrumented in CI: under AddressSanitizer and UBSan, under clang-tidy (`.clang-tidy`, findings are errors), and with coverage of `minipsd.h`: **lines must stay at - 100 %** and branches at 95 % or more, so new code comes with the test that executes it, + 100 %** and branches at 97 % or more, so new code comes with the test that executes it, error paths included (`test_alloc_failure.cpp` fails each allocation in turn). - **Single file:** the same tests also run against the single-file `minipsd.h` the release attaches, and `minipsd.h` is compiled alone and then with its implementation, with no internal diff --git a/include/minipsd.h b/include/minipsd.h index a63f0ee..f1b0687 100644 --- a/include/minipsd.h +++ b/include/minipsd.h @@ -35,7 +35,8 @@ * \par Configuration * Define before the implementation include: * - \c MINIPSD_STATIC: make every function static (private to that file); - * - \c MINIPSD_MALLOC / \c MINIPSD_FREE: replace malloc / free (define both); + * - \c MINIPSD_MALLOC / \c MINIPSD_FREE: replace malloc / free (define both; minipsd_read needs + * blocks aligned to 8 bytes, as malloc's are); * - \c MINIPSD_NO_MALLOC: leave out every allocation: minipsd_write, minipsd_read and * minipsd_doc_free go (and ); the pull writer, the reader and minipsd_read_into, on * the caller's memory, stay; @@ -158,7 +159,8 @@ enum { MINIPSD_MAX_SIDE_PSB), or the document is too large for PSD's 32-bit section lengths (about 4 GiB). */ MINIPSD_ERR_WRITE = -3, /*!< The write callback reported a failure. */ - MINIPSD_ERR_ALLOC = -4, /*!< MINIPSD_MALLOC returned NULL. */ + MINIPSD_ERR_ALLOC = -4, /*!< MINIPSD_MALLOC returned NULL, or for minipsd_read a + block aligned to less than 8 bytes. */ MINIPSD_ERR_OPEN = -5, /*!< Not returned by minipsd; kept for the caller's own file code (examples/): a file not opened or closed. */ MINIPSD_ERR_FORMAT = -6, /*!< Reader and typed views: the bytes are not well-formed. */ @@ -1049,7 +1051,9 @@ MINIPSDDEF int minipsd_reader_workspace_size(size_t *size); * thumbnail), and nothing after; for layers in a global block (see minipsd_info.layer_tail), up * to their count in that block. A buffer that ends sooner gives MINIPSD_ERR_SHORT: read more and * call again, on the same workspace; minipsd_reader_needed says how much. Nothing is allocated or - * copied, and calling init again restarts every iteration. + * copied, and calling init again restarts every iteration. What follows is checked as it is + * read: a record or channel data that overruns its section is minipsd_reader_next_layer's + * MINIPSD_ERR_FORMAT, so MINIPSD_OK here does not mean the whole file is sound. * * \param r Receives the reader, built inside workspace; set on MINIPSD_OK and on * MINIPSD_ERR_SHORT, NULL otherwise. @@ -1744,7 +1748,8 @@ MINIPSDDEF int minipsd_read_into(const void *data, size_t size, const minipsd_li * \brief Reads a whole file into a document in one MINIPSD_MALLOC block; minipsd_doc_free frees it. * * minipsd_read_size and minipsd_read_into on a block of its own: the same document, the same - * return codes, and MINIPSD_ERR_ALLOC when the allocation fails. + * return codes, and MINIPSD_ERR_ALLOC when the allocation fails or its block is aligned to less + * than 8 bytes (malloc's never is): the document must start the block for minipsd_doc_free. */ MINIPSDDEF int minipsd_read(const void *data, size_t size, const minipsd_limits *limits, const minipsd_read_opts *opts, minipsd_doc **doc, uint32_t *lost); diff --git a/include/minipsd/read.h b/include/minipsd/read.h index cc26404..17e7533 100644 --- a/include/minipsd/read.h +++ b/include/minipsd/read.h @@ -185,7 +185,7 @@ static int mpsd__read(const void *data, size_t size, const minipsd_limits *limit int k, keep; unsigned lplanes; /* decoded: transparency only when the file has it */ rc = minipsd_reader_next_layer(r, &v); - if (rc != MINIPSD_MORE) return rc == MINIPSD_OK ? MINIPSD_ERR_FORMAT : rc; + if (rc != MINIPSD_MORE) return rc == MINIPSD_OK ? MINIPSD_ERR_FORMAT : rc; /* OK: never, init counted them */ if (v.other_channels) loss |= MINIPSD_LOST_CHANNELS; /* TODO: alpha and spot channels (ROADMAP, Not scheduled) */ w = (uint64_t)((int64_t)v.right - v.left); h = (uint64_t)((int64_t)v.bottom - v.top); diff --git a/include/minipsd/reader.h b/include/minipsd/reader.h index d1b2da0..6b42ebe 100644 --- a/include/minipsd/reader.h +++ b/include/minipsd/reader.h @@ -313,7 +313,7 @@ MINIPSDDEF int minipsd_reader_init(minipsd_reader **out, const void *data, size_ r->data = (const uint8_t *)data; r->size = size; - r->base = 0; + r->base = 0; /* the window starts the file; nothing moves it yet (ROADMAP, streaming read) */ r->needed_from = r->needed = 0; r->limits.max_side = limits ? limits->max_side : 0; /* 0: the format's own, known from the header */ r->limits.max_layers = limits && limits->max_layers ? limits->max_layers : MPSD__MAX_LAYERS; diff --git a/include/minipsd/writer.h b/include/minipsd/writer.h index 98664f6..c7a8afe 100644 --- a/include/minipsd/writer.h +++ b/include/minipsd/writer.h @@ -789,7 +789,7 @@ static int mpsd__own_layers_block(const minipsd_doc *d) none. */ static int mpsd__alpha_uncounted(const minipsd_doc *d) { - return mpsd__composite_alpha(d) && !mpsd__doc_flat(d) && (mpsd__own_layers_block(d) || !d->layer_count); + return mpsd__composite_alpha(d) && (mpsd__own_layers_block(d) || !d->layer_count); } /* Then the writer names it as Photoshop does, which GIMP reads it by, unless the document keeps diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 05ac779..bad36dd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -23,6 +23,7 @@ if(NOT GTest_FOUND) set(INSTALL_GTEST OFF CACHE BOOL "" FORCE) FetchContent_Declare(GTest URL https://github.com/google/googletest/archive/refs/tags/v1.14.0.zip + URL_HASH SHA256=1f357c27ca988c3f7c6b4bf68a9395005ac6761f034046e9dde0896e3aba00e4 DOWNLOAD_EXTRACT_TIMESTAMP TRUE ) FetchContent_MakeAvailable(GTest) diff --git a/tests/test_alloc_failure.cpp b/tests/test_alloc_failure.cpp index 472590e..598a1d0 100644 --- a/tests/test_alloc_failure.cpp +++ b/tests/test_alloc_failure.cpp @@ -1,7 +1,8 @@ /** * \file - * \brief minipsd.h built with an allocator that fails on demand, so every MINIPSD_ERR_ALLOC path - * runs (its own translation unit: MINIPSD_MALLOC must be set before the implementation). + * \brief minipsd.h built with an allocator that fails on demand, or aligns less than malloc, so + * every MINIPSD_ERR_ALLOC path runs (its own translation unit: MINIPSD_MALLOC must be set + * before the implementation). * * SPDX-License-Identifier: MIT * SPDX-FileCopyrightText: Copyright (c) 2026 Bartlomiej Mucha @@ -13,19 +14,26 @@ #include namespace { -int g_allocs_left = -1; // allocations that still succeed; -1: all of them +int g_allocs_left = -1; // allocations that still succeed; -1: all of them +std::size_t g_misalign = 0; // bytes past malloc's address: a pool that aligns less void *failingMalloc(std::size_t size) { if (g_allocs_left == 0) return nullptr; if (g_allocs_left > 0) --g_allocs_left; - return std::malloc(size); + auto *p = static_cast(std::malloc(size + g_misalign)); + return p ? p + g_misalign : nullptr; +} + +void misalignedFree(void *ptr) +{ + std::free(ptr ? static_cast(ptr) - g_misalign : nullptr); } } // namespace #define MINIPSD_STATIC #define MINIPSD_MALLOC(size) failingMalloc(size) -#define MINIPSD_FREE(ptr) std::free(ptr) +#define MINIPSD_FREE(ptr) misalignedFree(ptr) #define MINIPSD_IMPLEMENTATION #include "minipsd.h" @@ -71,4 +79,13 @@ TEST(MinipsdAllocTest, EachFailedAllocationIsReported) ASSERT_EQ(minipsd_read(file, n, nullptr, nullptr, &doc, nullptr), MINIPSD_OK) << "one allocation is all a read needs"; minipsd_doc_free(doc); g_allocs_left = -1; + + // Blocks aligned to less than the document needs: the writer aligns inside its workspace, but + // minipsd_read's document could not start its block, so minipsd_doc_free could not free it. + g_misalign = MPSD__ALIGN / 2; + EXPECT_EQ(minipsd_write(&d, nullptr, count, &size), MINIPSD_OK) << "the writer takes any alignment"; + doc = reinterpret_cast(1); + EXPECT_EQ(minipsd_read(file, n, nullptr, nullptr, &doc, nullptr), MINIPSD_ERR_ALLOC); + EXPECT_EQ(doc, nullptr); + g_misalign = 0; } diff --git a/tests/test_reader.cpp b/tests/test_reader.cpp index d92913f..0fbfaf2 100644 --- a/tests/test_reader.cpp +++ b/tests/test_reader.cpp @@ -1498,8 +1498,9 @@ int countingInflate(void *user, const std::uint8_t *src, std::size_t size, std:: minipsd_target targetOf(Bytes &out, std::uint32_t w, int depth) { minipsd_target t; - if (depth == 16) minipsd_target_rgba16(&t, reinterpret_cast(out.data()), std::size_t(w) * 8); - else minipsd_target_rgba(&t, out.data(), std::size_t(w) * 4); + if (depth == 16) minipsd_target_rgba16(&t, reinterpret_cast(out.data()), std::size_t(w) * 8); + else if (depth == 32) minipsd_target_interleaved(&t, out.data(), std::size_t(w) * 16, 4, depth); + else minipsd_target_rgba(&t, out.data(), std::size_t(w) * 4); return t; } From 40aee19e0a781acc28c3a5617139932e6eb70b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Mucha?= Date: Wed, 4 Nov 2026 01:16:39 +0100 Subject: [PATCH 4/4] fix(ci): tidy and coverage on PR #16 - writer: clang-analyzer 22 (CI's) took the mask channel's source for a layer without a mask, after the crafted writer tests; mpsd__channel_count counts that channel only with a mask, so the finding is suppressed with that reason - ci: the C++ compile checks' (*_check.cpp, built, never run) coverage data left out, as the C units' are: their functions, never called, listed beside the tests' under their C names (functions 85 -> 100 %) Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 6 ++++-- docs/CHEATSHEET.md | 6 ++++-- include/minipsd/writer.h | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5ce4846..82cdbdf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -149,11 +149,13 @@ jobs: # --exclude-throw-branches: the C++ builds add exception edges that C code never takes. # The C units (the C smoke test, the examples, the fuzz replay) check that the header builds # and behaves as C, not coverage: gcc numbers a line's branches differently in C, so gcovr - # cannot merge them with the C++ units' and lists them twice. Their data goes first. + # cannot merge them with the C++ units' and lists them twice. Their data goes first, and so + # do the C++ compile checks' (*_check.cpp, built, never run): their functions, never called, + # would be listed beside the tests' under their C names. - name: Coverage report if: matrix.preset == 'linux-coverage' run: | - find build/linux-coverage \( -name '*.c.gcda' -o -name '*.c.gcno' \) -delete + find build/linux-coverage \( -name '*.c.gcda' -o -name '*.c.gcno' -o -name '*_check.cpp.gcno' \) -delete gcovr --root . --filter include/ --merge-lines --exclude-throw-branches \ --fail-under-line 100 --fail-under-branch 97 \ --cobertura coverage.xml --txt coverage.txt --txt-metric branch --txt-summary \ diff --git a/docs/CHEATSHEET.md b/docs/CHEATSHEET.md index 9861a95..9d69878 100644 --- a/docs/CHEATSHEET.md +++ b/docs/CHEATSHEET.md @@ -42,7 +42,7 @@ cmake --preset linux-asan && cmake --build --preset linux-asan && ctest --preset cmake --preset linux-tidy && cmake --build --preset linux-tidy # findings fail the build cmake --preset linux-coverage && cmake --build --preset linux-coverage && ctest --preset linux-coverage -find build/linux-coverage \( -name '*.c.gcda' -o -name '*.c.gcno' \) -delete # the C units: see below +find build/linux-coverage \( -name '*.c.gcda' -o -name '*.c.gcno' -o -name '*_check.cpp.gcno' \) -delete # see below gcovr --root . --filter include/ --merge-lines --exclude-throw-branches \ --txt-metric branch --txt build/linux-coverage # lines and branches missing a test ``` @@ -52,7 +52,9 @@ gcovr --root . --filter include/ --merge-lines --exclude-throw-branches \ The C units (the C smoke test, the examples, the fuzz replay) are left out, as CI leaves them out: they check the header builds and behaves as C, and gcc numbers a line's branches differently in C, so gcovr would list those lines twice and the duplicates would hide which test -is missing. Coverage data adds up across runs: delete `build/linux-coverage` for a clean report. +is missing. So are the C++ compile checks (`*_check.cpp`, built, never run): their functions, +never called, would show as missing a test. Coverage data adds up across runs: delete +`build/linux-coverage` for a clean report. CI fails below 100 % of lines or 97 % of branches and publishes the report to Coveralls. ## Fuzzing diff --git a/include/minipsd/writer.h b/include/minipsd/writer.h index c7a8afe..6b65339 100644 --- a/include/minipsd/writer.h +++ b/include/minipsd/writer.h @@ -225,7 +225,8 @@ static mpsd__source mpsd__channel_source(const minipsd_doc *d, const minipsd_lay src.height = r.height; } else if (k == mpsd__ch_mask(d)) { const minipsd_mask *m = l->mask; - const uint32_t w = (uint32_t)((int64_t)m->right - m->left), h = (uint32_t)((int64_t)m->bottom - m->top); + /* NOLINTNEXTLINE(clang-analyzer-core.NullDereference): mpsd__channel_count counts this channel only with a mask */ + const uint32_t w =(uint32_t)((int64_t)m->right - m->left), h = (uint32_t)((int64_t)m->bottom - m->top); src.height = h; if (m->stored.size) { src.stored = &m->stored; return src; } if (m->pixels && w && h) { src.samples = m->pixels; src.width = w; }