This is an automated email from the ASF dual-hosted git repository.
erickguan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opendal.git
The following commit(s) were added to refs/heads/main by this push:
new 29d55e8b1 Fix C binding empty byte buffers (#7807)
29d55e8b1 is described below
commit 29d55e8b1ff22e2252f34216f351480dd14fdd1e
Author: Minh Vu <[email protected]>
AuthorDate: Sun Jun 28 15:20:06 2026 +0200
Fix C binding empty byte buffers (#7807)
* Fix C binding empty byte buffers
* bindings/c: require null data for empty buffers
* bindings/c: clarify empty write_with test
---
bindings/c/include/opendal.h | 12 +++--
bindings/c/src/operator.rs | 12 ++++-
bindings/c/src/types.rs | 40 +++++++++++-----
bindings/c/src/writer.rs | 9 ++++
bindings/c/tests/bdd.cpp | 108 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 163 insertions(+), 18 deletions(-)
diff --git a/bindings/c/include/opendal.h b/bindings/c/include/opendal.h
index e3cad86d9..c835d980c 100644
--- a/bindings/c/include/opendal.h
+++ b/bindings/c/include/opendal.h
@@ -92,9 +92,11 @@ typedef struct opendal_presigned_request_inner
opendal_presigned_request_inner;
/**
* \brief opendal_bytes carries raw-bytes with its length
*
- * The opendal_bytes type is a C-compatible substitute for Vec type
- * in Rust, it has to be manually freed. You have to call opendal_bytes_free()
- * to free the heap memory to avoid memory leak.
+ * The opendal_bytes type is a C-compatible substitute for Vec type in Rust.
+ * For buffers returned by OpenDAL C APIs, call opendal_bytes_free() to free
+ * the heap memory and avoid memory leaks. For caller-owned input buffers
+ * passed to OpenDAL C APIs, the caller keeps ownership and must not call
+ * opendal_bytes_free() on them.
*
* @see opendal_bytes_free
*/
@@ -1490,8 +1492,8 @@ struct opendal_result_operator_new
opendal_operator_new_with_layers(const char *
* It is **safe** under the cases below
* * The memory pointed to by `path` must contain a valid nul terminator at
the end of
* the string.
- * * The `bytes` provided has valid byte in the `data` field and the `len`
field is set
- * correctly.
+ * * If `bytes.len` is greater than 0, `bytes.data` must point to at least
+ * `bytes.len` valid bytes. If `bytes.len` is 0, `bytes.data` must be NULL.
*
* # Panic
*
diff --git a/bindings/c/src/operator.rs b/bindings/c/src/operator.rs
index b8209be2e..ef2480818 100644
--- a/bindings/c/src/operator.rs
+++ b/bindings/c/src/operator.rs
@@ -259,8 +259,8 @@ pub unsafe extern "C" fn opendal_operator_new_with_layers(
/// It is **safe** under the cases below
/// * The memory pointed to by `path` must contain a valid nul terminator at
the end of
/// the string.
-/// * The `bytes` provided has valid byte in the `data` field and the `len`
field is set
-/// correctly.
+/// * If `bytes.len` is greater than 0, `bytes.data` must point to at least
+/// `bytes.len` valid bytes. If `bytes.len` is 0, `bytes.data` must be NULL.
///
/// # Panic
///
@@ -275,6 +275,10 @@ pub unsafe extern "C" fn opendal_operator_write(
let path = std::ffi::CStr::from_ptr(path)
.to_str()
.expect("malformed path");
+ let bytes = match bytes.to_buffer() {
+ Ok(bytes) => bytes,
+ Err(e) => return opendal_error::new(e),
+ };
match op.deref().write(path, bytes) {
Ok(_) => std::ptr::null_mut(),
Err(e) => opendal_error::new(e),
@@ -298,6 +302,10 @@ pub unsafe extern "C" fn opendal_operator_write_with(
} else {
(&*opts).into()
};
+ let bytes = match bytes.to_buffer() {
+ Ok(bytes) => bytes,
+ Err(e) => return opendal_error::new(e),
+ };
match op.deref().write_options(path, bytes, opts) {
Ok(_) => std::ptr::null_mut(),
Err(e) => opendal_error::new(e),
diff --git a/bindings/c/src/types.rs b/bindings/c/src/types.rs
index 7bbd787d4..df1fad2a6 100644
--- a/bindings/c/src/types.rs
+++ b/bindings/c/src/types.rs
@@ -21,8 +21,8 @@ use std::os::raw::c_char;
use opendal::options;
use opendal::raw::Timestamp;
-use opendal::Buffer;
use opendal::BytesRange;
+use opendal::{Buffer, Error, ErrorKind};
/// \brief Frees a heap-allocated string returned by OpenDAL C APIs.
///
@@ -36,9 +36,11 @@ pub unsafe extern "C" fn opendal_string_free(ptr: *mut
c_char) {
/// \brief opendal_bytes carries raw-bytes with its length
///
-/// The opendal_bytes type is a C-compatible substitute for Vec type
-/// in Rust, it has to be manually freed. You have to call opendal_bytes_free()
-/// to free the heap memory to avoid memory leak.
+/// The opendal_bytes type is a C-compatible substitute for Vec type in Rust.
+/// For buffers returned by OpenDAL C APIs, call opendal_bytes_free() to free
+/// the heap memory and avoid memory leaks. For caller-owned input buffers
+/// passed to OpenDAL C APIs, the caller keeps ownership and must not call
+/// opendal_bytes_free() on them.
///
/// @see opendal_bytes_free
#[repr(C)]
@@ -85,6 +87,29 @@ impl opendal_bytes {
}
}
}
+
+ pub(crate) fn to_buffer(&self) -> opendal::Result<Buffer> {
+ if self.len == 0 {
+ if self.data.is_null() {
+ return Ok(Buffer::new());
+ }
+
+ return Err(Error::new(
+ ErrorKind::Unexpected,
+ "empty opendal_bytes must have null data",
+ ));
+ }
+
+ if self.data.is_null() {
+ return Err(Error::new(
+ ErrorKind::Unexpected,
+ "non-empty opendal_bytes has null data",
+ ));
+ }
+
+ let slice = unsafe { std::slice::from_raw_parts(self.data, self.len) };
+ Ok(Buffer::from(bytes::Bytes::copy_from_slice(slice)))
+ }
}
/// \brief The options for the list operation.
@@ -1425,13 +1450,6 @@ impl Drop for opendal_bytes {
}
}
-impl From<&opendal_bytes> for Buffer {
- fn from(v: &opendal_bytes) -> Self {
- let slice = unsafe { std::slice::from_raw_parts(v.data, v.len) };
- Buffer::from(bytes::Bytes::copy_from_slice(slice))
- }
-}
-
/// \brief The configuration for the initialization of opendal_operator.
///
/// \note This is also a heap-allocated struct, please free it after you use it
diff --git a/bindings/c/src/writer.rs b/bindings/c/src/writer.rs
index 2b0eafa05..b5fbffd2b 100644
--- a/bindings/c/src/writer.rs
+++ b/bindings/c/src/writer.rs
@@ -52,6 +52,15 @@ impl opendal_writer {
bytes: &opendal_bytes,
) -> opendal_result_writer_write {
let size = bytes.len;
+ let bytes = match bytes.to_buffer() {
+ Ok(bytes) => bytes,
+ Err(e) => {
+ return opendal_result_writer_write {
+ size: 0,
+ error: opendal_error::new(e),
+ }
+ }
+ };
match self.deref_mut().write(bytes) {
Ok(()) => opendal_result_writer_write {
size,
diff --git a/bindings/c/tests/bdd.cpp b/bindings/c/tests/bdd.cpp
index 946df0c86..f9818c93b 100644
--- a/bindings/c/tests/bdd.cpp
+++ b/bindings/c/tests/bdd.cpp
@@ -138,3 +138,111 @@ TEST_F(OpendalBddTest, FeatureTest)
error = opendal_operator_delete(this->p, "tmpdir/");
EXPECT_EQ(error, nullptr);
}
+
+TEST_F(OpendalBddTest, WriteEmptyNullBytes)
+{
+ const opendal_bytes empty = {
+ .data = nullptr,
+ .len = 0,
+ .capacity = 0,
+ };
+
+ opendal_error* error = opendal_operator_write(this->p, "empty", &empty);
+ EXPECT_EQ(error, nullptr);
+
+ opendal_result_read read = opendal_operator_read(this->p, "empty");
+ EXPECT_EQ(read.error, nullptr);
+ EXPECT_EQ(read.data.len, 0);
+ opendal_bytes_free(&read.data);
+
+ // Cover the separate write_with entry point that shares this validation.
+ error = opendal_operator_write_with(this->p, "empty-with-options", &empty,
nullptr);
+ EXPECT_EQ(error, nullptr);
+
+ read = opendal_operator_read(this->p, "empty-with-options");
+ EXPECT_EQ(read.error, nullptr);
+ EXPECT_EQ(read.data.len, 0);
+ opendal_bytes_free(&read.data);
+
+ opendal_result_operator_writer writer = opendal_operator_writer(this->p,
"empty-writer");
+ EXPECT_EQ(writer.error, nullptr);
+ ASSERT_NE(writer.writer, nullptr);
+
+ opendal_result_writer_write write = opendal_writer_write(writer.writer,
&empty);
+ EXPECT_EQ(write.error, nullptr);
+ EXPECT_EQ(write.size, 0);
+
+ error = opendal_writer_close(writer.writer);
+ EXPECT_EQ(error, nullptr);
+ opendal_writer_free(writer.writer);
+
+ read = opendal_operator_read(this->p, "empty-writer");
+ EXPECT_EQ(read.error, nullptr);
+ EXPECT_EQ(read.data.len, 0);
+ opendal_bytes_free(&read.data);
+
+ EXPECT_EQ(opendal_operator_delete(this->p, "empty"), nullptr);
+ EXPECT_EQ(opendal_operator_delete(this->p, "empty-with-options"), nullptr);
+ EXPECT_EQ(opendal_operator_delete(this->p, "empty-writer"), nullptr);
+}
+
+TEST_F(OpendalBddTest, RejectNonEmptyNullBytes)
+{
+ const opendal_bytes invalid = {
+ .data = nullptr,
+ .len = 1,
+ .capacity = 0,
+ };
+
+ opendal_error* error = opendal_operator_write(this->p, "invalid",
&invalid);
+ ASSERT_NE(error, nullptr);
+ EXPECT_EQ(error->code, OPENDAL_UNEXPECTED);
+ opendal_error_free(error);
+
+ error = opendal_operator_write_with(this->p, "invalid-with-options",
&invalid, nullptr);
+ ASSERT_NE(error, nullptr);
+ EXPECT_EQ(error->code, OPENDAL_UNEXPECTED);
+ opendal_error_free(error);
+
+ opendal_result_operator_writer writer = opendal_operator_writer(this->p,
"invalid-writer");
+ EXPECT_EQ(writer.error, nullptr);
+ ASSERT_NE(writer.writer, nullptr);
+
+ opendal_result_writer_write write = opendal_writer_write(writer.writer,
&invalid);
+ EXPECT_EQ(write.size, 0);
+ ASSERT_NE(write.error, nullptr);
+ EXPECT_EQ(write.error->code, OPENDAL_UNEXPECTED);
+ opendal_error_free(write.error);
+ opendal_writer_free(writer.writer);
+}
+
+TEST_F(OpendalBddTest, RejectEmptyNonNullBytes)
+{
+ uint8_t sentinel = 0;
+ const opendal_bytes invalid = {
+ .data = &sentinel,
+ .len = 0,
+ .capacity = 0,
+ };
+
+ opendal_error* error = opendal_operator_write(this->p, "invalid-empty",
&invalid);
+ ASSERT_NE(error, nullptr);
+ EXPECT_EQ(error->code, OPENDAL_UNEXPECTED);
+ opendal_error_free(error);
+
+ error = opendal_operator_write_with(this->p, "invalid-empty-with-options",
&invalid, nullptr);
+ ASSERT_NE(error, nullptr);
+ EXPECT_EQ(error->code, OPENDAL_UNEXPECTED);
+ opendal_error_free(error);
+
+ opendal_result_operator_writer writer = opendal_operator_writer(this->p,
"invalid-empty-writer");
+ EXPECT_EQ(writer.error, nullptr);
+ ASSERT_NE(writer.writer, nullptr);
+
+ opendal_result_writer_write write = opendal_writer_write(writer.writer,
&invalid);
+ EXPECT_EQ(write.size, 0);
+ ASSERT_NE(write.error, nullptr);
+ EXPECT_EQ(write.error->code, OPENDAL_UNEXPECTED);
+ opendal_error_free(write.error);
+ opendal_writer_free(writer.writer);
+}
