Re: [PR] feat(binding/go): Add Copier [opendal]

["erickguan (via GitHub)" <gi...@apache.org>]

erickguan commented on code in PR #7732:
URL: https://github.com/apache/opendal/pull/7732#discussion_r3518233035


##########
bindings/c/include/opendal.h:
##########
@@ -784,6 +784,42 @@ typedef struct opendal_copy_options {
   uintptr_t chunk;
 } opendal_copy_options;
 
+/**
+ * \brief BlockingCopier is designed to drive a long-running copy operation in a
+ * blocking manner.
+ *
+ * Users can construct a copier by `opendal_operator_copier` or
+ * `opendal_operator_copier_with`. Each `opendal_copier_next` call drives the copy
+ * forward by one step and reports the number of bytes copied; the copy is complete
+ * once `has_next` is false.
+ *
+ * @see opendal_operator_copier()
+ */
+typedef struct opendal_copier {
+  /**
+   * The pointer to the opendal::blocking::Copier in the Rust code.
+   * Only touch this on judging whether it is NULL.
+   */
+  void *inner;
+} opendal_copier;
+
+/**
+ * \brief The result type returned by opendal_operator_copier().

Review Comment:
   minor: can use bullet-points to write comments about fields.



##########
bindings/c/include/opendal.h:
##########
@@ -784,6 +784,42 @@ typedef struct opendal_copy_options {
   uintptr_t chunk;
 } opendal_copy_options;
 
+/**
+ * \brief BlockingCopier is designed to drive a long-running copy operation in a
+ * blocking manner.
+ *
+ * Users can construct a copier by `opendal_operator_copier` or
+ * `opendal_operator_copier_with`. Each `opendal_copier_next` call drives the copy
+ * forward by one step and reports the number of bytes copied; the copy is complete

Review Comment:
   Is step well defined in C binding? Mentioning one-step to me is difficult to understand.



##########
bindings/c/include/opendal.h:
##########
@@ -2102,6 +2160,57 @@ struct opendal_error *opendal_operator_copy_with(const struct opendal_operator *
 
 struct opendal_error *opendal_operator_check(const struct opendal_operator *op);
 
+/**
+ * \brief Blocking create a copier to copy a file from `src` to `dest`.
+ *
+ * The returned copier drives a long-running copy operation. Call
+ * `opendal_copier_next` repeatedly to make progress, and `opendal_copier_free`
+ * to release it once finished.
+ *
+ * @param op The opendal_operator created previously
+ * @param src The designated source path you want to copy
+ * @param dest The designated destination path you want to copy
+ * @see opendal_operator
+ * @see opendal_copier
+ * @see opendal_result_operator_copier
+ * @return opendal_result_operator_copier, containing a copier and an opendal_error.
+ * If the operation succeeds, the `copier` field holds a valid copier and the `error`
+ * field is null. Otherwise, the `copier` will be null and the `error` will be set
+ * correspondingly.
+ *
+ * # Safety
+ *
+ * It is **safe** under the cases below
+ * * The memory pointed to by `src` and `dest` must contain a valid nul terminator at the
+ *   end of the string.
+ *
+ * # Panic
+ *
+ * * If the `src` or `dest` points to NULL, this function panics
+ */
+struct opendal_result_operator_copier opendal_operator_copier(const struct opendal_operator *op,

Review Comment:
   👍 



##########
bindings/c/include/opendal.h:
##########
@@ -1131,6 +1167,28 @@ typedef struct opendal_result_writer_write {
   struct opendal_error *error;
 } opendal_result_writer_write;
 
+/**
+ * \brief The result type returned by opendal_copier_next().
+ * The result type contains a `size` field, which is the number of bytes copied in this
+ * step (zero on error or completion), and a `has_next` field, which is true when the copy
+ * made progress and should be driven again. When `has_next` is false and `error` is null,

Review Comment:
   Add concept introduction on "drive".



##########
bindings/c/src/copier.rs:
##########
@@ -0,0 +1,105 @@
+// 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.
+
+use ::opendal as core;
+use std::ffi::c_void;
+
+use super::*;
+
+/// \brief BlockingCopier is designed to drive a long-running copy operation in a
+/// blocking manner.
+///
+/// Users can construct a copier by `opendal_operator_copier` or
+/// `opendal_operator_copier_with`. Each `opendal_copier_next` call drives the copy
+/// forward by one step and reports the number of bytes copied; the copy is complete
+/// once `has_next` is false.
+///
+/// @see opendal_operator_copier()
+#[repr(C)]
+pub struct opendal_copier {
+    /// The pointer to the opendal::blocking::Copier in the Rust code.
+    /// Only touch this on judging whether it is NULL.
+    inner: *mut c_void,
+}
+
+impl opendal_copier {
+    fn deref_mut(&mut self) -> &mut core::blocking::Copier {
+        // Safety: the inner should never be null once constructed
+        // The use-after-free is undefined behavior
+        unsafe { &mut *(self.inner as *mut core::blocking::Copier) }
+    }
+}
+
+impl opendal_copier {
+    pub(crate) fn new(copier: core::blocking::Copier) -> Self {
+        Self {
+            inner: Box::into_raw(Box::new(copier)) as _,
+        }
+    }
+
+    /// \brief Drive the copy operation forward by one step.
+    ///
+    /// Returns the number of bytes copied in this step. When `has_next` is true the
+    /// caller should keep calling this function to drive the copy. When `has_next` is
+    /// false and `error` is null the copy has completed.
+    ///
+    /// @see opendal_operator_copier()
+    #[no_mangle]
+    pub unsafe extern "C" fn opendal_copier_next(&mut self) -> opendal_result_copier_next {
+        match self.deref_mut().next() {
+            Some(Ok(n)) => opendal_result_copier_next {
+                size: n,
+                has_next: true,
+                error: std::ptr::null_mut(),
+            },
+            None => opendal_result_copier_next {
+                size: 0,
+                has_next: false,
+                error: std::ptr::null_mut(),
+            },
+            Some(Err(e)) => opendal_result_copier_next {
+                size: 0,
+                has_next: false,
+                error: opendal_error::new(e),
+            },
+        }
+    }
+
+    /// \brief Abort the pending copy operation.
+    ///
+    /// Returns NULL if the abort succeeds, otherwise it contains the error code and
+    /// error message.
+    #[no_mangle]
+    pub unsafe extern "C" fn opendal_copier_abort(&mut self) -> *mut opendal_error {
+        if let Err(e) = self.deref_mut().abort() {
+            opendal_error::new(e)
+        } else {
+            std::ptr::null_mut()
+        }
+    }
+
+    /// \brief Free the heap memory used by the opendal_copier.
+    #[no_mangle]
+    pub unsafe extern "C" fn opendal_copier_free(ptr: *mut opendal_copier) {
+        unsafe {
+            if !ptr.is_null() {
+                drop(Box::from_raw((*ptr).inner as *mut core::blocking::Copier));
+                drop(Box::from_raw(ptr));
+            }

Review Comment:
   set ptr = nullptr?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org