Merge branch 'main' into opendal_compat

.github/workflows/ci_bindings_c.yml

@@ -44,31 +44,20 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
-      - name: Install gtest manually
-        run: |
-          sudo apt-get update
-          sudo apt-get install libgtest-dev valgrind
-          cd /usr/src/gtest
-          sudo cmake CMakeLists.txt
-          sudo make
-          sudo cp lib/*.a /usr/lib
-          sudo ln -s /usr/lib/libgtest.a /usr/local/lib/libgtest.a
-          sudo ln -s /usr/lib/libgtest_main.a /usr/local/lib/libgtest_main.a
 
       - name: Setup Rust toolchain
         uses: ./.github/actions/setup
 
       - name: Build C binding
         working-directory: "bindings/c"
-        run: make build
+        run: |
+          mkdir build && cd build
+          cmake .. -DTEST_ENABLE_ASAN=ON
+          make -j$(nproc)
 
       - name: Check diff
         run: git diff --exit-code
 
       - name: Build and Run tests
         working-directory: "bindings/c"
-        run: make test
-
-      - name: Build and Run memory-leak tests
-        working-directory: "bindings/c"
-        run: make memory_leak
+        run: ./build/tests

bindings/c/CMakeLists.txt

@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+cmake_minimum_required(VERSION 3.22)
+project(opendal-c)
+
+if (NOT CMAKE_BUILD_TYPE)
+    set(CMAKE_BUILD_TYPE "Debug")
+endif()
+
+option(TEST_ENABLE_ASAN "Enable AddressSanitizer for tests" OFF)
+set(GOOGLETEST_VERSION 1.15.2)
+
+# force the compiler to support these standards
+set(CMAKE_C_STANDARD 11)
+set(CMAKE_C_STANDARD_REQUIRED ON)
+
+# for GoogleTest, it should be no less than C++14
+set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+
+# fetch google test via GitHub
+include(FetchContent)
+FetchContent_Declare(
+  googletest
+  URL https://github.com/google/googletest/archive/refs/tags/v${GOOGLETEST_VERSION}.zip
+)
+# For Windows: Prevent overriding the parent project's compiler/linker settings
+set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
+FetchContent_MakeAvailable(googletest)
+
+set(CARGO_DIST_DIR "${PROJECT_SOURCE_DIR}/target/debug")
+if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
+    set(CARGO_BUILD_TYPE "--release")
+    set(CARGO_DIST_DIR "${PROJECT_SOURCE_DIR}/target/release")
+endif()
+
+set(OPENDAL_STATIC_LIB "${CARGO_DIST_DIR}/libopendal_c.a")
+set(OPENDAL_SHARED_LIB "${CARGO_DIST_DIR}/libopendal_c.so")
+message(NOTICE "-- OpenDAL C static lib: ${OPENDAL_STATIC_LIB}")
+message(NOTICE "-- OpenDAL C shared lib: ${OPENDAL_SHARED_LIB}")
+
+# custom target for cargo build
+add_custom_target(cargo_build
+    COMMAND sh -c "cargo build ${CARGO_BUILD_TYPE}"
+    BYPRODUCTS  ${OPENDAL_STATIC_LIB} ${OPENDAL_SHARED_LIB}
+    WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+)
+
+# cmake target for static lib
+add_library(opendal_c_static INTERFACE)
+target_link_libraries(opendal_c_static INTERFACE ${OPENDAL_STATIC_LIB})
+target_include_directories(opendal_c_static INTERFACE include)
+add_dependencies(opendal_c_static cargo_build)
+
+# cmake target for shared lib
+add_library(opendal_c_shared INTERFACE)
+target_link_libraries(opendal_c_shared INTERFACE ${OPENDAL_SHARED_LIB})
+target_include_directories(opendal_c_shared INTERFACE include)
+add_dependencies(opendal_c_shared cargo_build)
+
+# example targets
+add_executable(basic examples/basic.c)
+target_link_libraries(basic opendal_c_shared)
+
+add_executable(error_handle examples/error_handle.c)
+target_link_libraries(error_handle opendal_c_shared)
+
+# test targets
+file(GLOB TEST_SRCS tests/*.cpp)
+add_executable(tests ${TEST_SRCS})
+target_link_libraries(tests opendal_c_shared gtest_main)
+if (TEST_ENABLE_ASAN)
+    target_compile_options(tests PRIVATE -fsanitize=address)
+    target_link_options(tests PRIVATE -fsanitize=address)
+endif()

bindings/c/Makefile

@@ -15,75 +15,17 @@
 # specific language governing permissions and limitations
 # under the License.
 
-RPATH=$(PWD)/target/debug
-OBJ_DIR=./build
 DOC_DIR=./docs
 
-CCFLAGS=-I./include
-CXXFLAGS=-I./include -std=c++14
-LDFLAGS=-L$(RPATH) -Wl,-rpath,$(RPATH)
-
-LIBS=-lopendal_c -lgtest -lpthread
-
-VALGRIND=valgrind --error-exitcode=1 --leak-check=full --
-
-.PHONY: all
-all: build test examples
-
 .PHONY: format
 format:
 	cargo fmt
 	find . -name '*.cpp' -exec clang-format -i --style=WebKit --verbose {} \;
 	find . -name '*.c' -exec clang-format -i --style=WebKit --verbose {} \;
 
-.PHONY: build
-build:
-	mkdir -p $(OBJ_DIR)
-	cargo build
-
-.PHONY: test
-test:
-	$(CXX) tests/bdd.cpp -o $(OBJ_DIR)/bdd $(CXXFLAGS) $(LDFLAGS) $(LIBS)
-	$(CXX) tests/list.cpp -o $(OBJ_DIR)/list $(CXXFLAGS) $(LDFLAGS) $(LIBS)
-	$(CXX) tests/error_msg.cpp -o $(OBJ_DIR)/error_msg $(CXXFLAGS) $(LDFLAGS) $(LIBS)
-	$(CXX) tests/opinfo.cpp -o $(OBJ_DIR)/opinfo $(CXXFLAGS) $(LDFLAGS) $(LIBS)
-	$(OBJ_DIR)/bdd
-	$(OBJ_DIR)/list
-	$(OBJ_DIR)/error_msg
-	$(OBJ_DIR)/opinfo
-
-.PHONY: test_memory_leak
-memory_leak:
-	$(VALGRIND) $(OBJ_DIR)/bdd
-	$(VALGRIND) $(OBJ_DIR)/list
-	$(VALGRIND) $(OBJ_DIR)/error_msg
-
 .PHONY: doc
 doc:
 	mkdir -p $(DOC_DIR)
 	curl --proto '=https' --tlsv1.2 -sSf https://cdn.jsdelivr.net/gh/jothepro/[email protected]/doxygen-awesome.min.css \
 		-o $(DOC_DIR)/doxygen-awesome.css
 	doxygen Doxyfile
-
-# build examples begin
-EXAMPLES=$(wildcard ./examples/*.c)
-EXAMPLE_OBJECTS=$(EXAMPLES:.c=.o)
-EXAMPLE_TARGETS=$(EXAMPLES:.c=)
-.PHONY: examples
-examples: $(EXAMPLE_TARGETS)
-
-$(EXAMPLE_TARGETS): % : %.o
-	$(CC) $(CCFLAGS) -o $@ $< $(LDFLAGS) $(LIBS)
-
-%.o: %.c
-	$(CC) $(CCFLAGS) -c $< -o $@
-# build examples end
-
-.PHONY: clean
-clean:
-	cargo clean
-	rm -rf $(EXAMPLE_OBJECTS)
-	rm -rf $(EXAMPLE_TARGETS)
-	rm -rf $(OBJ_DIR)
-	rm -rf $(DOC_DIR)
-

bindings/c/README.md

@@ -32,18 +32,18 @@ int main()
 
     /* We can read it out, make sure the data is the same */
     opendal_result_read r = opendal_operator_read(op, "/testpath");
-    opendal_bytes* read_bytes = r.data;
+    opendal_bytes read_bytes = r.data;
     assert(r.error == NULL);
-    assert(read_bytes->len == 24);
+    assert(read_bytes.len == 24);
 
     /* Lets print it out */
     for (int i = 0; i < 24; ++i) {
-        printf("%c", read_bytes->data[i]);
+        printf("%c", read_bytes.data[i]);
     }
     printf("\n");
 
     /* the opendal_bytes read is heap allocated, please free it */
-    opendal_bytes_free(read_bytes);
+    opendal_bytes_free(&read_bytes);
 
     /* the operator_ptr is also heap allocated */
     opendal_operator_free(&op);

bindings/c/examples/basic.c

@@ -37,23 +37,23 @@ int main()
     };
 
     /* Write this into path "/testpath" */
-    opendal_error* error = opendal_operator_write(op, "/testpath", data);
+    opendal_error* error = opendal_operator_write(op, "/testpath", &data);
     assert(error == NULL);
 
     /* We can read it out, make sure the data is the same */
     opendal_result_read r = opendal_operator_read(op, "/testpath");
-    opendal_bytes* read_bytes = r.data;
+    opendal_bytes read_bytes = r.data;
     assert(r.error == NULL);
-    assert(read_bytes->len == 24);
+    assert(read_bytes.len == 24);
 
     /* Lets print it out */
     for (int i = 0; i < 24; ++i) {
-        printf("%c", read_bytes->data[i]);
+        printf("%c", read_bytes.data[i]);
     }
     printf("\n");
 
     /* the opendal_bytes read is heap allocated, please free it */
-    opendal_bytes_free(read_bytes);
+    opendal_bytes_free(&read_bytes);
 
     /* the operator_ptr is also heap allocated */
     opendal_operator_free(op);

bindings/c/include/opendal.h

@@ -94,7 +94,7 @@ typedef struct opendal_bytes {
   /**
    * Pointing to the byte array on heap
    */
-  const uint8_t *data;
+  uint8_t *data;
   /**
    * The length of the byte array
    */
@@ -113,7 +113,7 @@ typedef struct opendal_bytes {
  * represents no error has taken placed**. If any error has taken place, the caller should check
  * the error code and print the error message.
  *
- * The error code is represented in opendal_code, which is a enum on different type of errors.
+ * The error code is represented in opendal_code, which is an enum on different type of errors.
  * The error messages is represented in opendal_bytes, which is a non-null terminated byte array.
  *
  * \note 1. The error message is on heap, so the error needs to be freed by the caller, by calling
@@ -217,7 +217,7 @@ typedef struct opendal_operator {
    * The pointer to the opendal::BlockingOperator in the Rust code.
    * Only touch this on judging whether it is NULL.
    */
-  const void *inner;
+  void *inner;
 } opendal_operator;
 
 /**
@@ -273,7 +273,7 @@ typedef struct opendal_result_read {
   /**
    * The byte array with length returned by read operations
    */
-  struct opendal_bytes *data;
+  struct opendal_bytes data;
   /**
    * The error, if ok, it is null
    */
@@ -814,7 +814,7 @@ struct opendal_result_operator_new opendal_operator_new(const char *scheme,
  */
 struct opendal_error *opendal_operator_write(const struct opendal_operator *op,
                                              const char *path,
-                                             struct opendal_bytes bytes);
+                                             const struct opendal_bytes *bytes);
 
 /**
  * \brief Blocking read the data from `path`.
@@ -842,8 +842,9 @@ struct opendal_error *opendal_operator_write(const struct opendal_operator *op,
  * opendal_result_read r = opendal_operator_read(op, "testpath");
  * assert(r.error == NULL);
  *
- * opendal_bytes *bytes = r.data;
- * assert(bytes->len == 13);
+ * opendal_bytes bytes = r.data;
+ * assert(bytes.len == 13);
+ * opendal_bytes_free(&bytes);
  * ```
  *
  * # Safety
@@ -1394,7 +1395,7 @@ void opendal_reader_free(struct opendal_reader *ptr);
  * \brief Write data to the writer.
  */
 struct opendal_result_writer_write opendal_writer_write(struct opendal_writer *self,
-                                                        struct opendal_bytes bytes);
+                                                        const struct opendal_bytes *bytes);
 
 /**
  * \brief Frees the heap memory used by the opendal_writer.

bindings/c/src/error.rs

@@ -82,7 +82,7 @@ impl From<core::ErrorKind> for opendal_code {
 /// represents no error has taken placed**. If any error has taken place, the caller should check
 /// the error code and print the error message.
 ///
-/// The error code is represented in opendal_code, which is a enum on different type of errors.
+/// The error code is represented in opendal_code, which is an enum on different type of errors.
 /// The error messages is represented in opendal_bytes, which is a non-null terminated byte array.
 ///
 /// \note 1. The error message is on heap, so the error needs to be freed by the caller, by calling
@@ -114,19 +114,7 @@ impl opendal_error {
     #[no_mangle]
     pub unsafe extern "C" fn opendal_error_free(ptr: *mut opendal_error) {
         if !ptr.is_null() {
-            let message_ptr = &(*ptr).message;
-            let message_ptr = message_ptr as *const opendal_bytes as *mut opendal_bytes;
-            if !message_ptr.is_null() {
-                let data_mut = (*message_ptr).data as *mut u8;
-                drop(Vec::from_raw_parts(
-                    data_mut,
-                    (*message_ptr).len,
-                    (*message_ptr).len,
-                ));
-            }
-
-            // free the pointer
-            drop(Box::from_raw(ptr))
+            drop(Box::from_raw(ptr));
         }
     }
 }