RDF 1.2.0

Author: Matthäus G. Chajdas

CMakeLists.txt

@@ -1,4 +1,4 @@
-### Copyright (c) 2021-2022 Advanced Micro Devices, Inc. All rights reserved. ###
+### Copyright (c) 2021-2023 Advanced Micro Devices, Inc. All rights reserved. ###
 cmake_minimum_required(VERSION 3.13..3.21)
 project(rdf LANGUAGES CXX C)
 
@@ -11,7 +11,7 @@ endif()
 option(RDF_BUILD_TESTS         "Build the tests"  ${RDF_IS_TOP_LEVEL})
 option(RDF_BUILD_TOOLS         "Build RDF tools"  ${RDF_IS_TOP_LEVEL})
 option(RDF_BUILD_INSTALL       "Install RDF libs" ${RDF_IS_TOP_LEVEL})
-option(RDF_ENABLE_CXX_BINDINGS "Allow usage of C++ interface for RDF library" ON)
+option(RDF_ENABLE_CXX_BINDINGS "Enable the C++ interface" ON)
 option(RDF_STATIC              "Build RDF as a static library" OFF)
 
 if(RDF_BUILD_TESTS)
@@ -40,7 +40,60 @@ if (RDF_IS_TOP_LEVEL)
     set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 endif()
 
-add_subdirectory(imported)
+################################################################################
+# Import third party libraries
+# For each library, three steps are checked to ensure it is present:
+#   1. Check if target is already provided
+#   2. Try to find lib via vcpkg
+#   3. Add target locally
+# We have to do this here for VCPKG to work, as we can't alias targets from
+# VCPKG in a subdirectory, and VCPKG uses different target names which we
+# don't want to force on everyone
+# We use QUIET on all finds because failure is an option (we just fall back
+# to our internal copy.)
+
+if(NOT TARGET zstd)
+    find_package(zstd QUIET)
+	if(TARGET zstd::libzstd_shared)
+		add_library(zstd ALIAS zstd::libzstd_shared)
+	elseif(TARGET zstd::libzstd_static)
+		add_library(zstd ALIAS zstd::libzstd_static)
+    else()
+	    add_subdirectory(imported/zstd)
+	endif()
+endif() 
+
+if(RDF_BUILD_TESTS AND (NOT TARGET catch2))
+    find_package(Catch2 QUIET)
+    if(TARGET Catch2::Catch2)
+		add_library(catch2 ALIAS Catch2::Catch2WithMain)
+    else()
+		add_subdirectory(imported/catch2)
+    endif()
+endif()
+
+if(RDF_BUILD_TOOLS)
+    if(NOT TARGET cli11)
+        find_package(CLI11 QUIET)
+        if(TARGET CLI11::CLI11)
+            add_library(cli11 ALIAS CLI11::CLI11)
+        else()
+            add_subdirectory(imported/cli11)
+        endif()
+    endif()
+
+    if(NOT TARGET json)
+        find_package(nlohmann_json QUIET)
+        if(TARGET nlohmann_json::nlohmann_json)
+            add_library(json ALIAS nlohmann_json::nlohmann_json)
+        else()
+            add_subdirectory(imported/json)
+        endif()
+    endif()
+endif()
+
+################################################################################
+# Project build
 add_subdirectory(rdf)
 if(RDF_BUILD_TOOLS)
     add_subdirectory(rdfi)

README.md

@@ -45,3 +45,8 @@ Patch releases (for example, `1.1.1`) will be bumped for bug fixes and other imp
 * **1.1.3**:
   * Fix missing includes on GCC13
   * Update `Zstd` dependency from 1.4.5 to 1.5.5
+* **1.2.0**
+  * Add support for [VCPKG](https://vcpkg.io/)
+    * The library can be now consumed via VCPKG
+    * The build can optionally use VCPKG to fetch dependencies
+  * Add a new `Close` callback function to `rdfUserStream`. This allows wrapped user streams to clean up the wrapped object, making it easier to track lifetime.

imported/CMakeLists.txt

@@ -26,7 +26,7 @@
      (static_cast<std::uint32_t>(minor) << 12) | \
      (static_cast<std::uint32_t>(patch)))
 
-#define RDF_INTERFACE_VERSION RDF_MAKE_VERSION(1, 1, 3)
+#define RDF_INTERFACE_VERSION RDF_MAKE_VERSION(1, 2, 0)
 
 extern "C" {
 struct rdfChunkFile;
@@ -70,13 +70,23 @@ struct rdfStreamFromFileCreateInfo
 /**
  * @brief User-provided I/O callbacks
  * 
- * There are five callback functions here which will be called with the user-
+ * There are six callback functions here which will be called with the user-
  * provided context variable:
  * 
  * - Seek/Tell/GetSize must be always non-null
  * - Read/Write can be null. Note that a stream which has both set to null
  *   is invalid. The chunk writer can work with a stream that is in write-
  *   only mode only if it's not appending.
+ * - Close can be null. This function will be called when the stream is closed,
+ *   which can be used to clean-up the context. A closed stream cannot be
+ *   reused, and it's the responsibility of the user to ensure a stream
+ *   is not closed more than once.
+ * 
+ * The first argument is always the context provided along with the user
+ * stream. Notice that the context pointer is copied internally, so you can't
+ * rely on the address of the context to be the same as the one passed into
+ * the various create functions. Specifically, you can't use `&ctx` to modify
+ * the context provided at creation time.
 */
 struct rdfUserStream
 {
@@ -137,11 +147,21 @@ struct rdfUserStream
      * 
      * This function must be always provided.
      * 
-     * -  `size` must not be `null`
+     * - `size` must not be `null`
      * - The provided context will be passed into `ctx`
     */
     int (*GetSize)(void* ctx, std::int64_t* size);
 
+    /**
+    * @brief Close the stream.
+    * @return rdfResult
+    * 
+    * This function can be `null` if the stream handles closing elsewhere.
+    * 
+    * @since 1.2
+    */
+    int (*Close)(void* ctx);
+
     void* context;
 };
 

imported/catch2/catch2/catch.hpp imported/catch2/catch.hppimported/catch2/catch2/catch.hpp renamed to imported/catch2/catch.hpp

@@ -1,7 +1,7 @@
 /* Copyright (c) 2021-2022 Advanced Micro Devices, Inc. All rights reserved. */
 #include "amdrdf.h"
 
-#include <zstd/zstd.h>
+#include <zstd.h>
 
 #include <cassert>
 #include <cstdio>
@@ -70,6 +70,8 @@ namespace internal
         bool CanWrite() const;
         bool CanRead() const;
 
+        void Close();
+
     private:
         virtual std::int64_t ReadImpl(const std::int64_t count, void* buffer) = 0;
         virtual std::int64_t WriteImpl(const std::int64_t count, const void* buffer) = 0;
@@ -80,6 +82,8 @@ namespace internal
 
         virtual bool CanWriteImpl() const = 0;
         virtual bool CanReadImpl() const = 0;
+
+        virtual void CloseImpl() = 0;
     };
 
     ///////////////////////////////////////////////////////////////////////////
@@ -155,6 +159,13 @@ namespace internal
             return stream_.Read != nullptr;
         }
 
+        void CloseImpl() override
+        {
+            if (stream_.Close) {
+                stream_.Close(stream_.context);
+            }
+        }
+
         void CheckCall(int result) const
         {
             if (result != rdfResultOk) {
@@ -812,6 +823,12 @@ namespace internal
         return WriteImpl(size, buffer);
     }
 
+    //////////////////////////////////////////////////////////////////////
+    void IStream::Close()
+    {
+        CloseImpl();
+    }
+    
     //////////////////////////////////////////////////////////////////////
     bool IStream::CanRead() const
     {
@@ -852,11 +869,6 @@ namespace internal
     public:
         Filestream(std::FILE* fd, rdfStreamAccess accessMode) : fd_(fd), accessMode_(accessMode) {}
 
-        ~Filestream()
-        {
-            std::fclose(fd_);
-        }
-
     private:
         std::int64_t ReadImpl(const std::int64_t count, void* buffer) override
         {
@@ -917,6 +929,12 @@ namespace internal
 #endif
         }
 
+        void CloseImpl() override
+        { 
+            std::fclose(fd_);
+            fd_ = nullptr;
+        }
+
         std::FILE* fd_;
         rdfStreamAccess accessMode_;
     };
@@ -990,6 +1008,13 @@ namespace internal
             return true;
         }
 
+        void CloseImpl() override
+        {
+            buffer_ = nullptr;
+            size_ = 0;
+            readPointer_ = 0;
+        }
+
         std::int64_t size_;
         std::size_t readPointer_ = 0;
         const void* buffer_;
@@ -1062,6 +1087,12 @@ namespace internal
             return true;
         }
 
+        void CloseImpl() override
+        { 
+            data_.clear();
+            offset_ = 0;
+        }
+
         std::vector<unsigned char> data_;
         std::size_t offset_ = 0;
     };
@@ -1383,6 +1414,8 @@ int RDF_EXPORT rdfStreamClose(rdfStream** handle)
         return rdfResult::rdfResultInvalidArgument;
     }
 
+    (*handle)->stream->Close();
+
     delete *handle;
     *handle = nullptr;
 

imported/cli11/cli11/CLI11.hpp imported/cli11/CLI/CLI.hppimported/cli11/cli11/CLI11.hpp renamed to imported/cli11/CLI/CLI.hpp

@@ -1,5 +1,5 @@
 /* Copyright (c) 2021-2022 Advanced Micro Devices, Inc. All rights reserved. */
-#include <catch2/catch.hpp>
+#include <catch.hpp>
 
 #include "amdrdf.h"
 
@@ -89,6 +89,8 @@ TEST_CASE("rdfUserStream", "[rdf]")
     {
         std::int64_t bytesRead = 0;
         std::int64_t bytesWritten = 0;
+
+        bool isClosed = false;
     } ctx;
 
     rdfUserStream us = {};
@@ -115,6 +117,11 @@ TEST_CASE("rdfUserStream", "[rdf]")
         return rdfResultOk;
     };
 
+    us.Close = [](void* context) -> int {
+        static_cast<Context*>(context)->isClosed = true;
+        return rdfResultOk;
+    };
+
     us.GetSize = [](void*, std::int64_t*) -> int { return rdfResultOk; };
     us.Tell = [](void*, std::int64_t*) -> int { return rdfResultOk; };
     us.Seek = [](void*, std::int64_t) -> int { return rdfResultOk; };
@@ -130,6 +137,9 @@ TEST_CASE("rdfUserStream", "[rdf]")
     CHECK(ctx.bytesWritten == 256);
 
     rdfStreamClose(&stream);
+
+    // Our close sets "isClosed"
+    CHECK(ctx.isClosed);
 }
 
 TEST_CASE("ChunkFileWriter with write-only stream", "[rdf]")
@@ -232,3 +242,49 @@ TEST_CASE("rdfUserStream required functions", "[rdf]")
         CHECK(stream == nullptr);
     }
 }
+
+TEST_CASE("rdfUserStream Close() can throw", "[rdf]")
+{
+    /*
+    Check that we can throw from Close(), and we can catch that, and still
+    repeat the Close() and correctly close the stream.
+    */
+
+    bool shouldThrow = true;
+
+    rdfUserStream us = {};
+    us.GetSize = [](void* ctx, int64_t* o) -> int {
+        return rdfResultOk;
+    };
+    us.Read = [](void* ctx, int64_t c, void* p, int64_t* o) -> int { 
+        return rdfResultOk;
+    };
+    us.Write = [](void* ctx, int64_t c, const void* p, int64_t* o) -> int {
+        return rdfResultOk;
+    };
+    us.Seek = [](void* ctx, int64_t o) -> int {
+        return rdfResultOk;
+    };
+    us.Tell = [](void* ctx, int64_t* p) -> int {
+        *p = 0;
+        return rdfResultOk;
+    };
+    us.Close = [](void* ctx) -> int {
+        if (*static_cast<bool*>(ctx)) {
+            throw 23;
+        }
+
+        return rdfResultOk;
+    };
+    us.context = &shouldThrow;
+
+    rdfStream* stream;
+    rdfStreamCreateFromUserStream(&us, &stream);
+
+    CHECK(rdfStreamClose(&stream) != rdfResultOk);
+    REQUIRE(stream != nullptr);
+
+    shouldThrow = false;
+    CHECK(rdfStreamClose(&stream) == rdfResultOk);
+    CHECK(stream == nullptr);
+}