ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Trade: disallow custom deleters in AbstractImporter::skinXD().

pull/470/head
Vladimír Vondruš 6 years ago
parent
8c24ea19d3
commit
5217f48499
5 changed files with 206 additions and 10 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 14
      src/Magnum/Trade/AbstractImporter.cpp
  2. 10
      src/Magnum/Trade/AbstractImporter.h
  3. 3
      src/Magnum/Trade/SkinData.cpp
  4. 5
      src/Magnum/Trade/SkinData.h
  5. 184
      src/Magnum/Trade/Test/AbstractImporterTest.cpp

14
src/Magnum/Trade/AbstractImporter.cpp
Unescape View File

@ -501,7 +501,12 @@ std::string AbstractImporter::doSkin2DName(UnsignedInt) { return {}; }
Containers::Optional<SkinData2D> AbstractImporter::skin2D(const UnsignedInt id) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::skin2D(): no file opened", {});
CORRADE_ASSERT(id < doSkin2DCount(), "Trade::AbstractImporter::skin2D(): index" << id << "out of range for" << doSkin2DCount() << "entries", {});
return doSkin2D(id);
Containers::Optional<SkinData2D> skin = doSkin2D(id);
CORRADE_ASSERT(!skin || (
(!skin->_jointData.deleter() || skin->_jointData.deleter() == reinterpret_cast<void(*)(UnsignedInt*, std::size_t)>(Implementation::nonOwnedArrayDeleter)) &&
(!skin->_inverseBindMatrixData.deleter() || skin->_inverseBindMatrixData.deleter() == reinterpret_cast<void(*)(Matrix3*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
"Trade::AbstractImporter::skin2D(): implementation is not allowed to use a custom Array deleter", {});
return skin;
}
Containers::Optional<SkinData2D> AbstractImporter::doSkin2D(UnsignedInt) {
@ -543,7 +548,12 @@ std::string AbstractImporter::doSkin3DName(UnsignedInt) { return {}; }
Containers::Optional<SkinData3D> AbstractImporter::skin3D(const UnsignedInt id) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::skin3D(): no file opened", {});
CORRADE_ASSERT(id < doSkin3DCount(), "Trade::AbstractImporter::skin3D(): index" << id << "out of range for" << doSkin3DCount() << "entries", {});
return doSkin3D(id);
Containers::Optional<SkinData3D> skin = doSkin3D(id);
CORRADE_ASSERT(!skin || (
(!skin->_jointData.deleter() || skin->_jointData.deleter() == reinterpret_cast<void(*)(UnsignedInt*, std::size_t)>(Implementation::nonOwnedArrayDeleter)) &&
(!skin->_inverseBindMatrixData.deleter() || skin->_inverseBindMatrixData.deleter() == reinterpret_cast<void(*)(Matrix4*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
"Trade::AbstractImporter::skin3D(): implementation is not allowed to use a custom Array deleter", {});
return skin;
}
Containers::Optional<SkinData3D> AbstractImporter::doSkin3D(UnsignedInt) {

10
src/Magnum/Trade/AbstractImporter.h
Unescape View File

@ -245,11 +245,11 @@ 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, @ref MaterialData and @ref MeshData are
only allowed to have default deleters (or be non-owning instances created from
@ref Corrade::Containers::ArrayView) --- this is to avoid potential dangling
function pointer calls when destructing such instances after the plugin module
has been unloaded.
@ref ImageData, @ref AnimationData, @ref MaterialData, @ref MeshData and
@ref SkinData are only allowed to have default deleters (or be non-owning
instances created from @ref Corrade::Containers::ArrayView) --- 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

3
src/Magnum/Trade/SkinData.cpp
Unescape View File

@ -28,6 +28,7 @@
#include "Magnum/Math/Matrix3.h"
#include "Magnum/Math/Matrix4.h"
#include "Magnum/Trade/Data.h"
#include "Magnum/Trade/Implementation/arrayUtilities.h"
namespace Magnum { namespace Trade {
@ -36,7 +37,7 @@ template<UnsignedInt dimensions> SkinData<dimensions>::SkinData(Containers::Arra
"Trade::SkinData: joint and inverse bind matrix arrays have different size, got" << _jointData.size() << "and" << _inverseBindMatrixData.size(), );
}
template<UnsignedInt dimensions> SkinData<dimensions>::SkinData(const std::initializer_list<UnsignedInt> joints, const std::initializer_list<MatrixTypeFor<dimensions, Float>> inverseBindMatrices, const void* const importerState): SkinData{Containers::array(joints), Containers::array(inverseBindMatrices), importerState} {}
template<UnsignedInt dimensions> SkinData<dimensions>::SkinData(const std::initializer_list<UnsignedInt> joints, const std::initializer_list<MatrixTypeFor<dimensions, Float>> inverseBindMatrices, const void* const importerState): SkinData{Implementation::initializerListToArrayWithDefaultDeleter(joints), Implementation::initializerListToArrayWithDefaultDeleter(inverseBindMatrices), importerState} {}
template<UnsignedInt dimensions> SkinData<dimensions>::SkinData(DataFlags, const Containers::ArrayView<const UnsignedInt> jointData, DataFlags, const Containers::ArrayView<const MatrixTypeFor<dimensions, Float>> inverseBindMatrixData, const void* const importerState) noexcept: SkinData<dimensions>{Containers::Array<UnsignedInt>{const_cast<UnsignedInt*>(jointData.data()), jointData.size(), reinterpret_cast<void(*)(UnsignedInt*, std::size_t)>(Implementation::nonOwnedArrayDeleter)}, Containers::Array<MatrixTypeFor<dimensions, Float>>{const_cast<MatrixTypeFor<dimensions, Float>*>(inverseBindMatrixData.data()), inverseBindMatrixData.size(), reinterpret_cast<void(*)(MatrixTypeFor<dimensions, Float>*, std::size_t)>(Implementation::nonOwnedArrayDeleter)}, importerState} {}

5
src/Magnum/Trade/SkinData.h
Unescape View File

@ -127,6 +127,11 @@ template<UnsignedInt dimensions> class SkinData {
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;
/** @todo skeleton object ID? gltf has that but the use is unclear */
Containers::Array<UnsignedInt> _jointData;
Containers::Array<MatrixTypeFor<dimensions, Float>> _inverseBindMatrixData;

184
src/Magnum/Trade/Test/AbstractImporterTest.cpp
Unescape View File

@ -147,12 +147,18 @@ struct AbstractImporterTest: TestSuite::Tester {
void skin2DNameOutOfRange();
void skin2DNotImplemented();
void skin2DOutOfRange();
void skin2DNonOwningDeleters();
void skin2DCustomJointDataDeleter();
void skin2DCustomInverseBindMatrixDataDeleter();
void skin3D();
void skin3DNameNotImplemented();
void skin3DNameOutOfRange();
void skin3DNotImplemented();
void skin3DOutOfRange();
void skin3DNonOwningDeleters();
void skin3DCustomJointDataDeleter();
void skin3DCustomInverseBindMatrixDataDeleter();
void mesh();
#ifdef MAGNUM_BUILD_DEPRECATED
@ -367,12 +373,18 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::skin2DNameOutOfRange,
&AbstractImporterTest::skin2DNotImplemented,
&AbstractImporterTest::skin2DOutOfRange,
&AbstractImporterTest::skin2DNonOwningDeleters,
&AbstractImporterTest::skin2DCustomJointDataDeleter,
&AbstractImporterTest::skin2DCustomInverseBindMatrixDataDeleter,
&AbstractImporterTest::skin3D,
&AbstractImporterTest::skin3DNameNotImplemented,
&AbstractImporterTest::skin3DNameOutOfRange,
&AbstractImporterTest::skin3DNotImplemented,
&AbstractImporterTest::skin3DOutOfRange,
&AbstractImporterTest::skin3DNonOwningDeleters,
&AbstractImporterTest::skin3DCustomJointDataDeleter,
&AbstractImporterTest::skin3DCustomInverseBindMatrixDataDeleter,
&AbstractImporterTest::mesh,
#ifdef MAGNUM_BUILD_DEPRECATED
@ -2201,7 +2213,9 @@ void AbstractImporterTest::skin2D() {
return {};
}
Containers::Optional<SkinData2D> doSkin2D(UnsignedInt id) override {
if(id == 7) return SkinData2D{{}, {}, &state};
/* Verify that initializer list is converted to an array with
the default deleter and not something disallowed */
if(id == 7) return SkinData2D{{1}, {{}}, &state};
return {};
}
} importer;
@ -2293,6 +2307,88 @@ void AbstractImporterTest::skin2DOutOfRange() {
CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::skin2D(): index 8 out of range for 8 entries\n");
}
void AbstractImporterTest::skin2DNonOwningDeleters() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doSkin2DCount() const override { return 1; }
Int doSkin2DForName(const std::string&) override { return 0; }
Containers::Optional<SkinData2D> doSkin2D(UnsignedInt) override {
return SkinData2D{{}, jointData, {}, inverseBindMatrixData};
}
UnsignedInt jointData[1]{};
Matrix3 inverseBindMatrixData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
auto data = importer.skin2D(0);
CORRADE_COMPARE(data->joints(), importer.jointData);
CORRADE_COMPARE(data->inverseBindMatrices(), importer.inverseBindMatrixData);
}
void AbstractImporterTest::skin2DCustomJointDataDeleter() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doSkin2DCount() const override { return 1; }
Int doSkin2DForName(const std::string&) override { return 0; }
Containers::Optional<SkinData2D> doSkin2D(UnsignedInt) override {
return SkinData2D{Containers::Array<UnsignedInt>{jointData, 1, [](UnsignedInt*, std::size_t){}}, Containers::Array<Matrix3>{1}};
}
UnsignedInt jointData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
importer.skin2D(0);
importer.skin2D("");
CORRADE_COMPARE(out.str(),
"Trade::AbstractImporter::skin2D(): implementation is not allowed to use a custom Array deleter\n"
"Trade::AbstractImporter::skin2D(): implementation is not allowed to use a custom Array deleter\n");
}
void AbstractImporterTest::skin2DCustomInverseBindMatrixDataDeleter() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doSkin2DCount() const override { return 1; }
Int doSkin2DForName(const std::string&) override { return 0; }
Containers::Optional<SkinData2D> doSkin2D(UnsignedInt) override {
return SkinData2D{Containers::Array<UnsignedInt>{1}, Containers::Array<Matrix3>{inverseBindMatrixData, 1, [](Matrix3*, std::size_t){}}};
}
Matrix3 inverseBindMatrixData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
importer.skin2D(0);
importer.skin2D("");
CORRADE_COMPARE(out.str(),
"Trade::AbstractImporter::skin2D(): implementation is not allowed to use a custom Array deleter\n"
"Trade::AbstractImporter::skin2D(): implementation is not allowed to use a custom Array deleter\n");
}
void AbstractImporterTest::skin3D() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
@ -2309,7 +2405,9 @@ void AbstractImporterTest::skin3D() {
return {};
}
Containers::Optional<SkinData3D> doSkin3D(UnsignedInt id) override {
if(id == 7) return SkinData3D{{}, {}, &state};
/* Verify that initializer list is converted to an array with
the default deleter and not something disallowed */
if(id == 7) return SkinData3D{{1}, {{}}, &state};
return {};
}
} importer;
@ -2401,6 +2499,88 @@ void AbstractImporterTest::skin3DOutOfRange() {
CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::skin3D(): index 8 out of range for 8 entries\n");
}
void AbstractImporterTest::skin3DNonOwningDeleters() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doSkin3DCount() const override { return 1; }
Int doSkin3DForName(const std::string&) override { return 0; }
Containers::Optional<SkinData3D> doSkin3D(UnsignedInt) override {
return SkinData3D{{}, jointData, {}, inverseBindMatrixData};
}
UnsignedInt jointData[1]{};
Matrix4 inverseBindMatrixData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
auto data = importer.skin3D(0);
CORRADE_COMPARE(data->joints(), importer.jointData);
CORRADE_COMPARE(data->inverseBindMatrices(), importer.inverseBindMatrixData);
}
void AbstractImporterTest::skin3DCustomJointDataDeleter() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doSkin3DCount() const override { return 1; }
Int doSkin3DForName(const std::string&) override { return 0; }
Containers::Optional<SkinData3D> doSkin3D(UnsignedInt) override {
return SkinData3D{Containers::Array<UnsignedInt>{jointData, 1, [](UnsignedInt*, std::size_t){}}, Containers::Array<Matrix4>{1}};
}
UnsignedInt jointData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
importer.skin3D(0);
importer.skin3D("");
CORRADE_COMPARE(out.str(),
"Trade::AbstractImporter::skin3D(): implementation is not allowed to use a custom Array deleter\n"
"Trade::AbstractImporter::skin3D(): implementation is not allowed to use a custom Array deleter\n");
}
void AbstractImporterTest::skin3DCustomInverseBindMatrixDataDeleter() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doSkin3DCount() const override { return 1; }
Int doSkin3DForName(const std::string&) override { return 0; }
Containers::Optional<SkinData3D> doSkin3D(UnsignedInt) override {
return SkinData3D{Containers::Array<UnsignedInt>{1}, Containers::Array<Matrix4>{inverseBindMatrixData, 1, [](Matrix4*, std::size_t){}}};
}
Matrix4 inverseBindMatrixData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
importer.skin3D(0);
importer.skin3D("");
CORRADE_COMPARE(out.str(),
"Trade::AbstractImporter::skin3D(): implementation is not allowed to use a custom Array deleter\n"
"Trade::AbstractImporter::skin3D(): implementation is not allowed to use a custom Array deleter\n");
}
void AbstractImporterTest::mesh() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }

Write Preview
Loading…
Cancel
Save