diff --git a/src/Magnum/Audio/AbstractImporter.cpp b/src/Magnum/Audio/AbstractImporter.cpp index ec92e68c3..8c75f29a7 100644 --- a/src/Magnum/Audio/AbstractImporter.cpp +++ b/src/Magnum/Audio/AbstractImporter.cpp @@ -127,7 +127,10 @@ UnsignedInt AbstractImporter::frequency() const { Containers::Array AbstractImporter::data() { CORRADE_ASSERT(isOpened(), "Audio::AbstractImporter::data(): no file opened", nullptr); - return doData(); + + Containers::Array out = doData(); + CORRADE_ASSERT(!out.deleter(), "Audio::AbstractImporter::data(): implementation is not allowed to use a custom Array deleter", {}); + return out; } Debug& operator<<(Debug& debug, const AbstractImporter::Feature value) { diff --git a/src/Magnum/Audio/AbstractImporter.h b/src/Magnum/Audio/AbstractImporter.h index 94d39c13b..c08ab05a1 100644 --- a/src/Magnum/Audio/AbstractImporter.h +++ b/src/Magnum/Audio/AbstractImporter.h @@ -43,6 +43,15 @@ Provides interface for importing various audio formats. See @ref plugins for more information and `*Importer` classes in @ref Audio namespace for available importer plugins. +@section Audio-AbstractImporter-data-dependency Data dependency + +The data returned from various functions *by design* have no dependency on the +importer instance and neither on the dynamic plugin module. In other words, you don't need to keep the importer instance (or the plugin manager instance) +around in order to have the returned data valid. Moreover, all returned +@ref Corrade::Containers::Array instances are only allowed to have default +deleters --- this is to avoid potential dangling function pointer calls when +destructing such instances after the plugin module has been unloaded. + @section Audio-AbstractImporter-subclassing Subclassing Plugin implements function @ref doFeatures(), @ref doIsOpened(), one of or both @@ -60,10 +69,15 @@ checked by the implementation: - All `do*()` implementations working on opened file are called only if there is any file opened. -@attention @ref Corrade::Containers::Array instances returned from the plugin - should *not* use anything else than the default deleter, otherwise this can - cause dangling function pointer call on array destruction if the plugin - gets unloaded before the array is destroyed. +@m_class{m-block m-warning} + +@par Dangling function pointers on plugin unload + As @ref Trade-AbstractImporter-data-dependency "mentioned above", + @ref Corrade::Containers::Array instances returned from plugin + implementations are not allowed to use anything else than the default + deleter, otherwise this could cause dangling function pointer call on array + destruction if the plugin gets unloaded before the array is destroyed. This + is asserted by the base implementation on return. */ class MAGNUM_AUDIO_EXPORT AbstractImporter: public PluginManager::AbstractManagingPlugin { public: diff --git a/src/Magnum/Audio/Test/AbstractImporterTest.cpp b/src/Magnum/Audio/Test/AbstractImporterTest.cpp index 8c3736d67..b4e552d12 100644 --- a/src/Magnum/Audio/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Audio/Test/AbstractImporterTest.cpp @@ -61,6 +61,7 @@ struct AbstractImporterTest: TestSuite::Tester { void data(); void dataNoFile(); + void dataCustomDeleter(); void debugFeature(); void debugFeatures(); @@ -85,6 +86,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::data, &AbstractImporterTest::dataNoFile, + &AbstractImporterTest::dataCustomDeleter, &AbstractImporterTest::debugFeature, &AbstractImporterTest::debugFeatures}); @@ -339,6 +341,26 @@ void AbstractImporterTest::dataNoFile() { CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::data(): no file opened\n"); } +void AbstractImporterTest::dataCustomDeleter() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { + return Containers::Array{nullptr, 0, [](char*, std::size_t) {}}; + } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.data(); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::data(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImporterTest::debugFeature() { std::ostringstream out; diff --git a/src/Magnum/Image.h b/src/Magnum/Image.h index 813c89bde..66c24124d 100644 --- a/src/Magnum/Image.h +++ b/src/Magnum/Image.h @@ -36,6 +36,11 @@ namespace Magnum { +#ifndef DOXYGEN_GENERATING_OUTPUT +/** @todo remove once AbstractImageConverter returns ImageData instead */ +namespace Trade { class AbstractImageConverter; } +#endif + /** @brief Image @@ -443,6 +448,14 @@ template class Image { Containers::Array release(); private: + #ifndef DOXYGEN_GENERATING_OUTPUT + /* For custom deleter checks. Not done in the constructors here because + the restriction is pointless when used outside of plugin + implementations. */ + /** @todo figure out a better way (return ImageData there instead?) */ + friend Trade::AbstractImageConverter; + #endif + PixelStorage _storage; PixelFormat _format; UnsignedInt _formatExtra; @@ -660,6 +673,14 @@ template class CompressedImage { Containers::Array release(); private: + #ifndef DOXYGEN_GENERATING_OUTPUT + /* For custom deleter checks. Not done in the constructors here because + the restriction is pointless when used outside of plugin + implementations. */ + /** @todo figure out a better way (return ImageData there instead?) */ + friend Trade::AbstractImageConverter; + #endif + /* To be made public once block size and block data size are stored together with the image */ explicit CompressedImage(CompressedPixelStorage storage, UnsignedInt format, const VectorTypeFor& size, Containers::Array&& data) noexcept; diff --git a/src/Magnum/Trade/AbstractImageConverter.cpp b/src/Magnum/Trade/AbstractImageConverter.cpp index 61bb51a6d..2cbd77c20 100644 --- a/src/Magnum/Trade/AbstractImageConverter.cpp +++ b/src/Magnum/Trade/AbstractImageConverter.cpp @@ -86,7 +86,9 @@ Containers::Optional AbstractImageConverter::exportToImage(const ImageV CORRADE_ASSERT(features() & Feature::ConvertImage, "Trade::AbstractImageConverter::exportToImage(): feature not supported", {}); - return doExportToImage(image); + Containers::Optional out = doExportToImage(image); + CORRADE_ASSERT(!out || !out->_data.deleter(), "Trade::AbstractImageConverter::exportToImage(): implementation is not allowed to use a custom Array deleter", {}); + return out; } Containers::Optional AbstractImageConverter::doExportToImage(const ImageView2D&) { @@ -97,7 +99,9 @@ Containers::Optional AbstractImageConverter::exportToCompress CORRADE_ASSERT(features() & Feature::ConvertCompressedImage, "Trade::AbstractImageConverter::exportToCompressedImage(): feature not supported", {}); - return doExportToCompressedImage(image); + Containers::Optional out = doExportToCompressedImage(image); + CORRADE_ASSERT(!out || !out->_data.deleter(), "Trade::AbstractImageConverter::exportToCompressedImage(): implementation is not allowed to use a custom Array deleter", {}); + return out; } Containers::Optional AbstractImageConverter::doExportToCompressedImage(const ImageView2D&) { @@ -108,7 +112,9 @@ Containers::Array AbstractImageConverter::exportToData(const ImageView2D& CORRADE_ASSERT(features() & Feature::ConvertData, "Trade::AbstractImageConverter::exportToData(): feature not supported", nullptr); - return doExportToData(image); + Containers::Array out = doExportToData(image); + CORRADE_ASSERT(!out.deleter(), "Trade::AbstractImageConverter::exportToData(): implementation is not allowed to use a custom Array deleter", {}); + return out; } Containers::Array AbstractImageConverter::doExportToData(const ImageView2D&) { @@ -119,7 +125,9 @@ Containers::Array AbstractImageConverter::exportToData(const CompressedIma CORRADE_ASSERT(features() & Feature::ConvertCompressedData, "Trade::AbstractImageConverter::exportToData(): feature not supported", nullptr); - return doExportToData(image); + Containers::Array out = doExportToData(image); + CORRADE_ASSERT(!out.deleter(), "Trade::AbstractImageConverter::exportToData(): implementation is not allowed to use a custom Array deleter", {}); + return out; } Containers::Array AbstractImageConverter::doExportToData(const CompressedImageView2D&) { diff --git a/src/Magnum/Trade/AbstractImageConverter.h b/src/Magnum/Trade/AbstractImageConverter.h index 4c05a90af..192897950 100644 --- a/src/Magnum/Trade/AbstractImageConverter.h +++ b/src/Magnum/Trade/AbstractImageConverter.h @@ -44,6 +44,17 @@ Provides functionality for converting images between various internal formats or compressing them. See @ref plugins for more information and `*ImageConverter` classes in @ref Trade namespace for available image converter plugins. +@section Trade-AbstractImageConverter-data-dependency Data dependency + +The instances returned from various functions *by design* have no dependency on +the importer instance and neither on the dynamic plugin module. In other words, +you don't need to keep the importer instance (or the plugin manager instance) +around in order to have the `*Data` instances valid. Moreover, all +@ref Corrade::Containers::Array instances returned through @ref Image, +@ref CompressedImage and others are only allowed to have default deleters --- +this is to avoid potential dangling function pointer calls when destructing +such instances after the plugin module has been unloaded. + @section Trade-AbstractImageConverter-subclassing Subclassing The plugin needs to implement the@ref doFeatures() function and one or more of @@ -63,10 +74,15 @@ checked by the implementation: - The function @ref doExportToData(const CompressedImageView2D&) is called only if @ref Feature::ConvertCompressedData is supported. -@attention @ref Corrade::Containers::Array instances returned from the plugin - should *not* use anything else than the default deleter, otherwise this can - cause dangling function pointer call on array destruction if the plugin - gets unloaded before the array is destroyed. +@m_class{m-block m-warning} + +@par Dangling function pointers on plugin unload + As @ref Trade-AbstractImageConverter-data-dependency "mentioned above", + @ref Corrade::Containers::Array instances returned from plugin + implementations are not allowed to use anything else than the default + deleter, otherwise this could cause dangling function pointer call on array + destruction if the plugin gets unloaded before the array is destroyed. This + is asserted by the base implementation on return. */ class MAGNUM_TRADE_EXPORT AbstractImageConverter: public PluginManager::AbstractManagingPlugin { public: diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index b83056bc6..b021580f2 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -265,7 +265,9 @@ std::string AbstractImporter::doAnimationName(UnsignedInt) { return {}; } Containers::Optional AbstractImporter::animation(const UnsignedInt id) { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::animation(): no file opened", {}); CORRADE_ASSERT(id < doAnimationCount(), "Trade::AbstractImporter::animation(): index" << id << "out of range for" << doAnimationCount() << "entries", {}); - return doAnimation(id); + Containers::Optional animation = doAnimation(id); + CORRADE_ASSERT(!animation || (!animation->_data.deleter() && !animation->_tracks.deleter()), "Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter", {}); + return animation; } Containers::Optional AbstractImporter::doAnimation(UnsignedInt) { @@ -553,7 +555,9 @@ std::string AbstractImporter::doImage1DName(UnsignedInt) { return {}; } Containers::Optional AbstractImporter::image1D(const UnsignedInt id) { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image1D(): no file opened", {}); CORRADE_ASSERT(id < doImage1DCount(), "Trade::AbstractImporter::image1D(): index" << id << "out of range for" << doImage1DCount() << "entries", {}); - return doImage1D(id); + Containers::Optional image = doImage1D(id); + CORRADE_ASSERT(!image || !image->_data.deleter(), "Trade::AbstractImporter::image1D(): implementation is not allowed to use a custom Array deleter", {}); + return image; } Containers::Optional AbstractImporter::doImage1D(UnsignedInt) { @@ -585,7 +589,9 @@ std::string AbstractImporter::doImage2DName(UnsignedInt) { return {}; } Containers::Optional AbstractImporter::image2D(const UnsignedInt id) { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image2D(): no file opened", {}); CORRADE_ASSERT(id < doImage2DCount(), "Trade::AbstractImporter::image2D(): index" << id << "out of range for" << doImage2DCount() << "entries", {}); - return doImage2D(id); + Containers::Optional image = doImage2D(id); + CORRADE_ASSERT(!image || !image->_data.deleter(), "Trade::AbstractImporter::image2D(): implementation is not allowed to use a custom Array deleter", {}); + return image; } Containers::Optional AbstractImporter::doImage2D(UnsignedInt) { @@ -617,7 +623,9 @@ std::string AbstractImporter::doImage3DName(UnsignedInt) { return {}; } Containers::Optional AbstractImporter::image3D(const UnsignedInt id) { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image3D(): no file opened", {}); CORRADE_ASSERT(id < doImage3DCount(), "Trade::AbstractImporter::image3D(): index" << id << "out of range for" << doImage3DCount() << "entries", {}); - return doImage3D(id); + Containers::Optional image = doImage3D(id); + CORRADE_ASSERT(!image || !image->_data.deleter(), "Trade::AbstractImporter::image3D(): implementation is not allowed to use a custom Array deleter", {}); + return image; } Containers::Optional AbstractImporter::doImage3D(UnsignedInt) { diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index 0c9bb3662..ef9c3624a 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -145,6 +145,24 @@ Another option is making use of the @ref Containers::pointerCast() utility, but note that in that case the original @ref Corrade::Containers::Pointer will be * *moved into* a new instance and that might not be desirable. +@section Trade-AbstractImporter-data-dependency Data dependency + +The `*Data` instances returned from various functions *by design* have no +dependency on the importer instance and neither on the dynamic plugin module. +In other words, you don't need to keep the importer instance (or the plugin +manager instance) around in order to have the `*Data` instances valid. +Moreover, all @ref Corrade::Containers::Array instances returned through +@ref ImageData, @ref AnimationData and others are only allowed to have default +deleters --- this is to avoid potential dangling function pointer calls when +destructing such instances after the plugin module has been unloaded. + +The only exception are various `importerState()` functions +@ref Trade-AbstractImporter-usage-state "described above", but in that case the +relation is *weak* --- these are valid only as long as the currently opened +file is kept open. If the file gets closed or the importer instance deleted, +the state pointers become dangling, and that's fine as long as you don't access +them. + @section Trade-AbstractImporter-subclassing Subclassing The plugin needs to implement the @ref doFeatures(), @ref doIsOpened() @@ -183,19 +201,25 @@ checked by the implementation: - All `do*()` implementations taking data ID as parameter are called only if the ID is from valid range. -@attention - @ref Corrade::Containers::Array instances returned from the plugin - should *not* use anything else than the default deleter, otherwise this can - cause dangling function pointer call on array destruction if the plugin - gets unloaded before the array is destroyed. -@attention +@m_class{m-block m-warning} + +@par Dangling function pointers on plugin unload + As @ref Trade-AbstractImporter-data-dependency "mentioned above", + @ref Corrade::Containers::Array instances returned from plugin + implementations are not allowed to use anything else than the default + deleter, otherwise this could cause dangling function pointer call on array + destruction if the plugin gets unloaded before the array is destroyed. This + is asserted by the base implementation on return. +@par Similarly for interpolator functions passed through @ref Animation::TrackView instances to @ref AnimationData --- to avoid dangling pointers, be sure to always include an interpolator returned from @ref animationInterpolatorFor(), which guarantees the function is *not* instantiated in the plugin binary. Avoid using - @ref Animation::interpolatorFor() (or indirectly it by specifying - just @ref Animation::Interpolation), as it doesn't have such guarantee. + @ref Animation::interpolatorFor() (or indirectly using it by specifying + just @ref Animation::Interpolation), as it doesn't have such a guarantee. + Note that unlike with array instances, the base implementation can't easily + check for this. */ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagingPlugin { public: diff --git a/src/Magnum/Trade/AnimationData.h b/src/Magnum/Trade/AnimationData.h index a52b51540..7d9b27819 100644 --- a/src/Magnum/Trade/AnimationData.h +++ b/src/Magnum/Trade/AnimationData.h @@ -433,6 +433,11 @@ class MAGNUM_TRADE_EXPORT AnimationData { const void* importerState() const { return _importerState; } private: + /* For custom deleter checks. Not done in the constructors here because + the restriction is pointless when used outside of plugin + implementations. */ + friend AbstractImporter; + Range1D _duration; Containers::Array _data; Containers::Array _tracks; diff --git a/src/Magnum/Trade/ImageData.h b/src/Magnum/Trade/ImageData.h index 10efc8eed..ba6f70a1f 100644 --- a/src/Magnum/Trade/ImageData.h +++ b/src/Magnum/Trade/ImageData.h @@ -33,6 +33,7 @@ #include "Magnum/DimensionTraits.h" #include "Magnum/PixelStorage.h" +#include "Magnum/Trade/Trade.h" #include "Magnum/Trade/visibility.h" namespace Magnum { namespace Trade { @@ -427,6 +428,11 @@ template class ImageData { const void* importerState() const { return _importerState; } private: + /* For custom deleter checks. Not done in the constructors here because + the restriction is pointless when used outside of plugin + implementations. */ + friend AbstractImporter; + explicit ImageData(CompressedPixelStorage storage, UnsignedInt format, const VectorTypeFor& size, Containers::Array&& data, const void* importerState = nullptr) noexcept; bool _compressed; diff --git a/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp b/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp index 799d16d26..5b68e1e8f 100644 --- a/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImageConverterTest.cpp @@ -52,18 +52,22 @@ class AbstractImageConverterTest: public TestSuite::Tester { void exportToImage(); void exportToImageNotSupported(); void exportToImageNotImplemented(); + void exportToImageCustomDeleter(); void exportToCompressedImage(); void exportToCompressedImageNotSupported(); void exportToCompressedImageNotImplemented(); + void exportToCompressedImageCustomDeleter(); void exportToData(); void exportToDataNotSupported(); void exportToDataNotImplemented(); + void exportToDataCustomDeleter(); void exportCompressedToData(); void exportCompressedToDataNotSupported(); void exportCompressedToDataNotImplemented(); + void exportCompressedToDataCustomDeleter(); void exportImageDataToData(); @@ -92,18 +96,22 @@ AbstractImageConverterTest::AbstractImageConverterTest() { &AbstractImageConverterTest::exportToImage, &AbstractImageConverterTest::exportToImageNotSupported, &AbstractImageConverterTest::exportToImageNotImplemented, + &AbstractImageConverterTest::exportToImageCustomDeleter, &AbstractImageConverterTest::exportToCompressedImage, &AbstractImageConverterTest::exportToCompressedImageNotSupported, &AbstractImageConverterTest::exportToCompressedImageNotImplemented, + &AbstractImageConverterTest::exportToCompressedImageCustomDeleter, &AbstractImageConverterTest::exportToData, &AbstractImageConverterTest::exportToDataNotSupported, &AbstractImageConverterTest::exportToDataNotImplemented, + &AbstractImageConverterTest::exportToDataCustomDeleter, &AbstractImageConverterTest::exportCompressedToData, &AbstractImageConverterTest::exportCompressedToDataNotSupported, &AbstractImageConverterTest::exportCompressedToDataNotImplemented, + &AbstractImageConverterTest::exportCompressedToDataCustomDeleter, &AbstractImageConverterTest::exportImageDataToData, @@ -192,6 +200,22 @@ void AbstractImageConverterTest::exportToImageNotImplemented() { CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToImage(): feature advertised but not implemented\n"); } +void AbstractImageConverterTest::exportToImageCustomDeleter() { + class Converter: public AbstractImageConverter { + Features doFeatures() const override { return Feature::ConvertImage; } + Containers::Optional doExportToImage(const ImageView2D&) override { + return Image2D{PixelFormat::RGBA8Unorm, {}, Containers::Array{nullptr, 0, [](char*, std::size_t) {}}}; + } + }; + + std::ostringstream out; + Error redirectError{&out}; + + Converter converter; + converter.exportToImage(ImageView2D{PixelFormat::R8Unorm, {}}); + CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToImage(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImageConverterTest::exportToCompressedImage() { class Converter: public AbstractImageConverter { Features doFeatures() const override { return Feature::ConvertCompressedImage; } @@ -233,6 +257,22 @@ void AbstractImageConverterTest::exportToCompressedImageNotImplemented() { CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToCompressedImage(): feature advertised but not implemented\n"); } +void AbstractImageConverterTest::exportToCompressedImageCustomDeleter() { + class Converter: public AbstractImageConverter { + Features doFeatures() const override { return Feature::ConvertCompressedImage; } + Containers::Optional doExportToCompressedImage(const ImageView2D&) override { + return CompressedImage2D{CompressedPixelFormat::Bc1RGBAUnorm, {}, Containers::Array{nullptr, 0, [](char*, std::size_t) {}}}; + } + }; + + std::ostringstream out; + Error redirectError{&out}; + + Converter converter; + converter.exportToCompressedImage(ImageView2D{PixelFormat::R8Unorm, {}}); + CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToCompressedImage(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImageConverterTest::exportToData() { class Converter: public AbstractImageConverter { Features doFeatures() const override { return Feature::ConvertData; } @@ -272,6 +312,22 @@ void AbstractImageConverterTest::exportToDataNotImplemented() { CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToData(): feature advertised but not implemented\n"); } +void AbstractImageConverterTest::exportToDataCustomDeleter() { + class Converter: public AbstractImageConverter { + Features doFeatures() const override { return Feature::ConvertData; } + Containers::Array doExportToData(const ImageView2D&) override { + return Containers::Array{nullptr, 0, [](char*, std::size_t) {}}; + } + }; + + std::ostringstream out; + Error redirectError{&out}; + + Converter converter; + converter.exportToData(ImageView2D{PixelFormat::RGBA8Unorm, {}}); + CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToData(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImageConverterTest::exportCompressedToData() { class Converter: public AbstractImageConverter { Features doFeatures() const override { return Feature::ConvertCompressedData; } @@ -311,16 +367,42 @@ void AbstractImageConverterTest::exportCompressedToDataNotImplemented() { CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToData(): feature advertised but not implemented\n"); } +void AbstractImageConverterTest::exportCompressedToDataCustomDeleter() { + class Converter: public AbstractImageConverter { + Features doFeatures() const override { return Feature::ConvertCompressedData; } + Containers::Array doExportToData(const CompressedImageView2D&) override { + return Containers::Array{nullptr, 0, [](char*, std::size_t) {}}; + } + }; + + std::ostringstream out; + Error redirectError{&out}; + + Converter converter; + converter.exportToData(CompressedImageView2D{CompressedPixelFormat::Bc1RGBAUnorm, {}}); + CORRADE_COMPARE(out.str(), "Trade::AbstractImageConverter::exportToData(): implementation is not allowed to use a custom Array deleter\n"); +} + class ImageDataExporter: public Trade::AbstractImageConverter { private: Features doFeatures() const override { return Feature::ConvertData|Feature::ConvertCompressedData; } Containers::Array doExportToData(const ImageView2D&) override { - return Containers::Array{Containers::InPlaceInit, {'B'}}; + /* DirectInit / InPlaceInit is unfortunately causing a custom + deleter right now */ + /** @todo clean up when fixed */ + Containers::Array out{1}; + out[0] = 'B'; + return out; }; Containers::Array doExportToData(const CompressedImageView2D&) override { - return Containers::Array{Containers::InPlaceInit, {'C'}}; + /* DirectInit / InPlaceInit is unfortunately causing a custom + deleter right now */ + /** @todo clean up when fixed */ + Containers::Array out{1}; + out[0] = 'C'; + return out; }; }; diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 5f33c6843..40c8f4b8c 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -105,6 +105,8 @@ struct AbstractImporterTest: TestSuite::Tester { void animationNotImplemented(); void animationNoFile(); void animationOutOfRange(); + void animationCustomDataDeleter(); + void animationCustomTrackDeleter(); void light(); void lightCountNotImplemented(); @@ -213,6 +215,7 @@ struct AbstractImporterTest: TestSuite::Tester { void image1DNotImplemented(); void image1DNoFile(); void image1DOutOfRange(); + void image1DCustomDeleter(); void image2D(); void image2DCountNotImplemented(); @@ -225,6 +228,7 @@ struct AbstractImporterTest: TestSuite::Tester { void image2DNotImplemented(); void image2DNoFile(); void image2DOutOfRange(); + void image2DCustomDeleter(); void image3D(); void image3DCountNotImplemented(); @@ -237,6 +241,7 @@ struct AbstractImporterTest: TestSuite::Tester { void image3DNotImplemented(); void image3DNoFile(); void image3DOutOfRange(); + void image3DCustomDeleter(); void importerState(); void importerStateNotImplemented(); @@ -300,6 +305,8 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::animationNotImplemented, &AbstractImporterTest::animationNoFile, &AbstractImporterTest::animationOutOfRange, + &AbstractImporterTest::animationCustomDataDeleter, + &AbstractImporterTest::animationCustomTrackDeleter, &AbstractImporterTest::light, &AbstractImporterTest::lightCountNotImplemented, @@ -408,6 +415,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::image1DNotImplemented, &AbstractImporterTest::image1DNoFile, &AbstractImporterTest::image1DOutOfRange, + &AbstractImporterTest::image1DCustomDeleter, &AbstractImporterTest::image2D, &AbstractImporterTest::image2DCountNotImplemented, @@ -420,6 +428,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::image2DNotImplemented, &AbstractImporterTest::image2DNoFile, &AbstractImporterTest::image2DOutOfRange, + &AbstractImporterTest::image2DCustomDeleter, &AbstractImporterTest::image3D, &AbstractImporterTest::image3DCountNotImplemented, @@ -432,6 +441,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::image3DNotImplemented, &AbstractImporterTest::image3DNoFile, &AbstractImporterTest::image3DOutOfRange, + &AbstractImporterTest::image3DCustomDeleter, &AbstractImporterTest::importerState, &AbstractImporterTest::importerStateNotImplemented, @@ -1295,6 +1305,44 @@ void AbstractImporterTest::animationOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::animation(): index 8 out of range for 8 entries\n"); } +void AbstractImporterTest::animationCustomDataDeleter() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doAnimationCount() const override { return 1; } + Containers::Optional doAnimation(UnsignedInt) override { + return AnimationData{Containers::Array{nullptr, 0, [](char*, std::size_t) {}}, nullptr}; + } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.animation(0); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter\n"); +} + +void AbstractImporterTest::animationCustomTrackDeleter() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doAnimationCount() const override { return 1; } + Containers::Optional doAnimation(UnsignedInt) override { + return AnimationData{nullptr, Containers::Array{nullptr, 0, [](AnimationTrackData*, std::size_t) {}}}; + } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.animation(0); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImporterTest::light() { struct: AbstractImporter { Features doFeatures() const override { return {}; } @@ -2813,6 +2861,25 @@ void AbstractImporterTest::image1DOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image1D(): index 8 out of range for 8 entries\n"); } +void AbstractImporterTest::image1DCustomDeleter() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doImage1DCount() const override { return 1; } + Containers::Optional doImage1D(UnsignedInt) override { + return ImageData1D{PixelFormat::RGBA8Unorm, {}, Containers::Array{nullptr, 0, [](char*, std::size_t) {}}}; + } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.image1D(0); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image1D(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImporterTest::image2D() { struct: AbstractImporter { Features doFeatures() const override { return {}; } @@ -2979,6 +3046,25 @@ void AbstractImporterTest::image2DOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image2D(): index 8 out of range for 8 entries\n"); } +void AbstractImporterTest::image2DCustomDeleter() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doImage2DCount() const override { return 1; } + Containers::Optional doImage2D(UnsignedInt) override { + return ImageData2D{PixelFormat::RGBA8Unorm, {}, Containers::Array{nullptr, 0, [](char*, std::size_t) {}}}; + } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.image2D(0); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image2D(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImporterTest::image3D() { struct: AbstractImporter { Features doFeatures() const override { return {}; } @@ -3145,6 +3231,25 @@ void AbstractImporterTest::image3DOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image3D(): index 8 out of range for 8 entries\n"); } +void AbstractImporterTest::image3DCustomDeleter() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doImage3DCount() const override { return 1; } + Containers::Optional doImage3D(UnsignedInt) override { + return ImageData3D{PixelFormat::RGBA8Unorm, {}, Containers::Array{nullptr, 0, [](char*, std::size_t) {}}}; + } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.image3D(0); + CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image3D(): implementation is not allowed to use a custom Array deleter\n"); +} + void AbstractImporterTest::importerState() { struct: AbstractImporter { Features doFeatures() const override { return {}; }