[WIP] Code cleanup and some new unittests for edgecases.

code/Common/AssertHandler.cpp

@@ -5,8 +5,6 @@ Open Asset Import Library (assimp)
 
 Copyright (c) 2006-2022, assimp team
 
-
-
 All rights reserved.
 
 Redistribution and use of this software in source and binary forms,
@@ -50,23 +48,23 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <iostream>
 #include <cstdlib>
 
-void Assimp::defaultAiAssertHandler(const char* failedExpression, const char* file, int line)
-{
+void Assimp::defaultAiAssertHandler(const char* failedExpression, const char* file, int line) {
     std::cerr << "ai_assert failure in " << file << "(" << line << "): " << failedExpression << std::endl;
     std::abort();
 }
 
-namespace
-{
+namespace {
     Assimp::AiAssertHandler s_handler = Assimp::defaultAiAssertHandler;
 }
 
-void Assimp::setAiAssertHandler(AiAssertHandler handler)
-{
-    s_handler = handler;
+void Assimp::setAiAssertHandler(AiAssertHandler handler) {
+    if (handler != nullptr) {
+        s_handler = handler;
+    } else {
+        s_handler = Assimp::defaultAiAssertHandler;
+    }
 }
 
-void Assimp::aiAssertViolation(const char* failedExpression, const char* file, int line)
-{
+void Assimp::aiAssertViolation(const char* failedExpression, const char* file, int line) {
     s_handler(failedExpression, file, line);
 }

code/Common/AssertHandler.h

@@ -47,29 +47,33 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <assimp/ai_assert.h>
 #include <assimp/defs.h>
 
-namespace Assimp
-{
-    // ---------------------------------------------------------------------------
-    /** Signature of functions which handle assert violations.
-     */
-    using AiAssertHandler = void (*)(const char* failedExpression, const char* file, int line);
-
-    // ---------------------------------------------------------------------------
-    /** Set the assert handler.
-     */
-    ASSIMP_API void setAiAssertHandler(AiAssertHandler handler);
-
-    // ---------------------------------------------------------------------------
-    /** The assert handler which is set by default.
-     *
-     * This issues a message to stderr and calls abort.
-     */
-    ASSIMP_API void defaultAiAssertHandler(const char* failedExpression, const char* file, int line);
-
-    // ---------------------------------------------------------------------------
-    /** Dispatches an assert violation to the assert handler.
-     */
-    ASSIMP_API void aiAssertViolation(const char* failedExpression, const char* file, int line);
+namespace Assimp {
+
+// ---------------------------------------------------------------------------
+/**
+ *  @brief  Signature of functions which handle assert violations.
+ */
+using AiAssertHandler = void (*)(const char* failedExpression, const char* file, int line);
+
+// ---------------------------------------------------------------------------
+/**
+ *  @brief  Set the assert handler.
+ */
+ASSIMP_API void setAiAssertHandler(AiAssertHandler handler);
+
+// ---------------------------------------------------------------------------
+/** The assert handler which is set by default.
+ *
+ *  @brief  This issues a message to stderr and calls abort.
+ */
+ASSIMP_API void defaultAiAssertHandler(const char* failedExpression, const char* file, int line);
+
+// ---------------------------------------------------------------------------
+/**
+ *  @brief  Dispatches an assert violation to the assert handler.
+ */
+ASSIMP_API void aiAssertViolation(const char* failedExpression, const char* file, int line);
+
 } // end of namespace Assimp
 
-#endif // INCLUDED_AI_ASSERTHANDLER_H
+#endif // INCLUDED_AI_ASSERTHANDLER_H

code/Common/Base64.cpp

@@ -46,7 +46,7 @@ namespace Assimp {
 
 namespace Base64 {
 
-static const uint8_t tableDecodeBase64[128] = {
+static constexpr uint8_t tableDecodeBase64[128] = {
     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 62, 0, 0, 0, 63,
@@ -57,7 +57,7 @@ static const uint8_t tableDecodeBase64[128] = {
     41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 0, 0, 0, 0, 0
 };
 
-static const char *tableEncodeBase64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
+static constexpr char tableEncodeBase64[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
 
 static inline char EncodeChar(uint8_t b) {
     return tableEncodeBase64[size_t(b)];
@@ -71,6 +71,11 @@ inline uint8_t DecodeChar(char c) {
 }
 
 void Encode(const uint8_t *in, size_t inLength, std::string &out) {
+    if (in == nullptr || inLength==0) {
+        out.clear();
+        return;
+    }
+
     size_t outLength = ((inLength + 2) / 3) * 4;
 
     size_t j = out.size();
@@ -115,8 +120,14 @@ std::string Encode(const std::vector<uint8_t> &in) {
 }
 
 size_t Decode(const char *in, size_t inLength, uint8_t *&out) {
+    if (in == nullptr) {
+        out = nullptr;
+        return 0;
+    }
+
     if (inLength % 4 != 0) {
-        throw DeadlyImportError("Invalid base64 encoded data: \"", std::string(in, std::min(size_t(32), inLength)), "\", length:", inLength);
+        throw DeadlyImportError("Invalid base64 encoded data: \"", std::string(in, std::min(size_t(32), inLength)),
+            "\", length:", inLength);
     }
 
     if (inLength < 4) {

code/Common/BaseImporter.cpp

@@ -234,19 +234,23 @@ void BaseImporter::GetExtensionList(std::set<std::string> &extensions) {
     std::string::size_type pos = pFile.find_last_of('.');
 
     // no file extension - can't read
-    if (pos == std::string::npos)
+    if (pos == std::string::npos) {
         return false;
+    }
 
     const char *ext_real = &pFile[pos + 1];
-    if (!ASSIMP_stricmp(ext_real, ext0))
+    if (!ASSIMP_stricmp(ext_real, ext0)) {
         return true;
+    }
 
     // check for other, optional, file extensions
-    if (ext1 && !ASSIMP_stricmp(ext_real, ext1))
+    if (ext1 && !ASSIMP_stricmp(ext_real, ext1)) {
         return true;
+    }
 
-    if (ext2 && !ASSIMP_stricmp(ext_real, ext2))
-        return true;
+    if (ext2 && !ASSIMP_stricmp(ext_real, ext2)) {
+        return true;        
+    }
 
     return false;
 }

code/Common/BaseProcess.cpp

@@ -66,17 +66,26 @@ BaseProcess::~BaseProcess() {
 // ------------------------------------------------------------------------------------------------
 void BaseProcess::ExecuteOnScene(Importer *pImp) {
     ai_assert( nullptr != pImp );
-    ai_assert( nullptr != pImp->Pimpl()->mScene);
+    if (pImp == nullptr) {
+        return;
+    }
+
+    ai_assert(nullptr != pImp->Pimpl()->mScene);
+    if (pImp->Pimpl()->mScene == nullptr) {
+        return;
+    }
 
     progress = pImp->GetProgressHandler();
     ai_assert(nullptr != progress);
+    if (progress == nullptr) {
+        return;
+    }
 
     SetupProperties(pImp);
 
     // catch exceptions thrown inside the PostProcess-Step
     try {
         Execute(pImp->Pimpl()->mScene);
-
     } catch (const std::exception &err) {
 
         // extract error description

code/Common/BaseProcess.h

@@ -175,23 +175,24 @@ class SharedPostProcessInfo {
  * should be executed. If the function returns true, the class' Execute()
  * function is called subsequently.
  */
-class ASSIMP_API_WINONLY BaseProcess {
+class ASSIMP_API BaseProcess {
     friend class Importer;
 
 public:
-    /** Constructor to be privately used by Importer */
+    /** @brief onstructor to be privately used by Importer */
     BaseProcess() AI_NO_EXCEPT;
 
-    /** Destructor, private as well */
+    /** @brief Destructor, private as well */
     virtual ~BaseProcess();
 
     // -------------------------------------------------------------------
-    /** Returns whether the processing step is present in the given flag.
+    /**
+     * @brief Returns whether the processing step is present in the given flag.
      * @param pFlags The processing flags the importer was called with. A
      *   bitwise combination of #aiPostProcessSteps.
      * @return true if the process is present in this flag fields,
      *   false if not.
-    */
+     */
     virtual bool IsActive(unsigned int pFlags) const = 0;
 
     // -------------------------------------------------------------------
@@ -200,33 +201,36 @@ class ASSIMP_API_WINONLY BaseProcess {
     virtual bool RequireVerboseFormat() const;
 
     // -------------------------------------------------------------------
-    /** Executes the post processing step on the given imported data.
-    * The function deletes the scene if the postprocess step fails (
-    * the object pointer will be set to nullptr).
-    * @param pImp Importer instance (pImp->mScene must be valid)
-    */
+    /**
+     * @brief Executes the post processing step on the given imported data.
+     * The function deletes the scene if the post-process step fails (
+     * the object pointer will be set to nullptr).
+     * @param pImp Importer instance (pImp->mScene must be valid)
+     */
     void ExecuteOnScene(Importer *pImp);
 
     // -------------------------------------------------------------------
-    /** Called prior to ExecuteOnScene().
-    * The function is a request to the process to update its configuration
-    * basing on the Importer's configuration property list.
-    */
+    /**
+     * @brief Called prior to ExecuteOnScene().
+     * The function is a request to the process to update its configuration
+     * basing on the Importer's configuration property list.
+     */
     virtual void SetupProperties(const Importer *pImp);
 
     // -------------------------------------------------------------------
-    /** Executes the post processing step on the given imported data.
-    * A process should throw an ImportErrorException* if it fails.
-    * This method must be implemented by deriving classes.
-    * @param pScene The imported data to work at.
-    */
+    /**
+     * @brief Executes the post processing step on the given imported data.
+     * A process should throw an ImportErrorException* if it fails.
+     * This method must be implemented by deriving classes.
+     * @param pScene The imported data to work at.
+     */
     virtual void Execute(aiScene *pScene) = 0;
 
     // -------------------------------------------------------------------
     /** Assign a new SharedPostProcessInfo to the step. This object
-     *  allows multiple postprocess steps to share data.
+     *  allows multiple post-process steps to share data.
      * @param sh May be nullptr
-    */
+     */
     inline void SetSharedData(SharedPostProcessInfo *sh) {
         shared = sh;
     }

include/assimp/Base64.hpp

@@ -50,16 +50,38 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 namespace Assimp {
 namespace Base64 {
 
-/// @brief Will encode the given 
-/// @param in 
-/// @param inLength 
-/// @param out 
+/// @brief Will encode the given character buffer from UTF64 to ASCII
+/// @param in           The UTF-64 buffer.
+/// @param inLength     The size of the buffer
+/// @param out          The encoded ASCII string.
 void Encode(const uint8_t *in, size_t inLength, std::string &out);
+
+/// @brief Will encode the given character buffer from UTF64 to ASCII.
+/// @param in   A vector, which contains the buffer for encoding.
+/// @param out  The encoded ASCII string.
 void Encode(const std::vector<uint8_t>& in, std::string &out);
+
+/// @brief Will encode the given character buffer from UTF64 to ASCII.
+/// @param in   A vector, which contains the buffer for encoding.
+/// @return The encoded ASCII string.
 std::string Encode(const std::vector<uint8_t>& in);
 
+/// @brief Will decode the given character buffer from ASCII to UTF64.
+/// @param in           The ASCII buffer to decode.
+/// @param inLength     The size of the buffer.
+/// @param out          The decoded buffer.
+/// @return The new buffer size.
 size_t Decode(const char *in, size_t inLength, uint8_t *&out);
+
+/// @brief Will decode the given character buffer from ASCII to UTF64.
+/// @param in   The ASCII buffer to decode as a std::string.
+/// @param out  The decoded buffer.
+/// @return The new buffer size.
 size_t Decode(const std::string& in, std::vector<uint8_t>& out);
+
+/// @brief Will decode the given character buffer from ASCII to UTF64.
+/// @param in   The ASCII string.
+/// @return The decoded buffer in a vector.
 std::vector<uint8_t> Decode(const std::string& in);
 
 } // namespace Base64

test/CMakeLists.txt

@@ -71,7 +71,6 @@ SET( COMMON
   unit/AssimpAPITest_aiQuaternion.cpp
   unit/AssimpAPITest_aiVector2D.cpp
   unit/AssimpAPITest_aiVector3D.cpp
-  unit/Common/utHash.cpp
   unit/MathTest.cpp
   unit/MathTest.h
   unit/RandomNumberGeneration.h
@@ -98,6 +97,8 @@ SET( COMMON
   unit/Common/utAssertHandler.cpp
   unit/Common/utXmlParser.cpp
   unit/Common/utBase64.cpp
+  unit/Common/utHash.cpp
+  unit/Common/utBaseProcess.cpp
 )
 
 SET( IMPORTERS

test/unit/Common/utBase64.cpp

@@ -47,26 +47,35 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 using namespace std;
 using namespace Assimp;
 
-class Base64Test : public ::testing::Test {
-public:
-    virtual void SetUp() {
-    }
-
-    virtual void TearDown() {
-    }
-};
+class Base64Test : public ::testing::Test {};
 
 static const std::vector<uint8_t> assimpStringBinary = { 97, 115, 115, 105, 109, 112 };
 static const std::string assimpStringEncoded = "YXNzaW1w";
 
-TEST_F( Base64Test, encodeTest ) {
-    EXPECT_EQ( "", Base64::Encode (std::vector<uint8_t>{}) );
-    EXPECT_EQ( "Vg==", Base64::Encode (std::vector<uint8_t>{ 86 }) );
-    EXPECT_EQ( assimpStringEncoded, Base64::Encode (assimpStringBinary) );
+TEST_F( Base64Test, encodeTest) {
+    EXPECT_EQ( "", Base64::Encode(std::vector<uint8_t>{}) );
+    EXPECT_EQ( "Vg==", Base64::Encode(std::vector<uint8_t>{ 86 }) );
+    EXPECT_EQ( assimpStringEncoded, Base64::Encode(assimpStringBinary) );
+}
+
+TEST_F( Base64Test, encodeTestWithNullptr ) {
+    std::string out;
+    Base64::Encode(nullptr, 100u, out);
+    EXPECT_TRUE(out.empty());
+
+    Base64::Encode(&assimpStringBinary[0], 0u, out);
+    EXPECT_TRUE(out.empty());
+}
+
+TEST_F( Base64Test, decodeTest) {
+    EXPECT_EQ( std::vector<uint8_t> {}, Base64::Decode("") );
+    EXPECT_EQ( std::vector<uint8_t> { 86 }, Base64::Decode("Vg==") );
+    EXPECT_EQ( assimpStringBinary, Base64::Decode(assimpStringEncoded) );
 }
 
-TEST_F( Base64Test, decodeTest ) {
-    EXPECT_EQ( std::vector<uint8_t> {}, Base64::Decode ("") );
-    EXPECT_EQ( std::vector<uint8_t> { 86 }, Base64::Decode ("Vg==") );
-    EXPECT_EQ( assimpStringBinary, Base64::Decode (assimpStringEncoded) );
+TEST_F(Base64Test, decodeTestWithNullptr) {
+    uint8_t *out = nullptr;
+    size_t size = Base64::Decode(nullptr, 100u, out);
+    EXPECT_EQ(nullptr, out);
+    EXPECT_EQ(0u, size);
 }