[GitHub] [incubator-opendal] Xuanwo commented on a diff in pull request #1632: feat(bindings/c): basic read and write op, a dirty way

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

Xuanwo commented on code in PR #1632:
URL: https://github.com/apache/incubator-opendal/pull/1632#discussion_r1138012224


##########
bindings/c/cbindgen.toml:
##########


Review Comment:
   Do we still need this file now that `build.rs` has been introduced?



##########
bindings/c/include/opendal.h:
##########
@@ -17,25 +17,33 @@
  * under the License.
  */
 
+#ifndef __OPENDAL_H__
+#define __OPENDAL_H__
 
-#ifndef _OPENDAL_H
-#define _OPENDAL_H
+/* Do not modify. This code was autogenerated by bindgen. */
 
-#include <stdint.h>
-#include <stddef.h>
+#include <stdarg.h>
 #include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdlib.h>
 
-#ifdef __cplusplus
-extern "C" {
-#endif // __cplusplus
+typedef struct BlockingOperator BlockingOperator;

Review Comment:
   I have no knowledge about async/blocking in the C ecosystem. How do they handle naming conventions?



##########
bindings/c/include/opendal.h:
##########
@@ -17,25 +17,33 @@
  * under the License.
  */
 
+#ifndef __OPENDAL_H__
+#define __OPENDAL_H__
 
-#ifndef _OPENDAL_H
-#define _OPENDAL_H
+/* Do not modify. This code was autogenerated by bindgen. */
 
-#include <stdint.h>
-#include <stddef.h>
+#include <stdarg.h>
 #include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdlib.h>
 
-#ifdef __cplusplus
-extern "C" {
-#endif // __cplusplus
+typedef struct BlockingOperator BlockingOperator;
 
-/*
- Hello, OpenDAL!
+typedef struct DynArray {
+  uint8_t *array;
+  size_t length;
+} DynArray;
+
+/**
+ * Hello, OpenDAL!
  */
 void hello_opendal(void);
 
-#ifdef __cplusplus
-} // extern "C"
-#endif // __cplusplus
+struct BlockingOperator *blocking_op_create(void);
+
+struct DynArray blocking_read(const struct BlockingOperator *op, const char *path);
+
+void blocking_write(const struct BlockingOperator *op, const char *path, const struct DynArray *bs);

Review Comment:
   Is it common for C to use `DynArray` to write data?
   
   https://github.com/awslabs/aws-c-s3/blob/main/include/aws/s3/s3_client.h#L103-L105



##########
bindings/c/tests/test.c:
##########
@@ -18,7 +18,34 @@
  */
 #include "opendal.h"
 
-int main(int argc, char *argv[]) {
-    hello_opendal();
-    return 0;
+#include <stdio.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+static void test_hello_opendal(void **state)
+{
+	(void)state; /* unused */
+
+	hello_opendal();
+}
+
+static void test_blocking_op(void **state)
+{
+	(void)state; /* unused */
+
+	BlockingOperator *op = blocking_op_create();
+	DynArray wbs = {"hello, world!", 13};
+	blocking_write(op, "opendal", &wbs);
+	DynArray rbs = blocking_read(op, "opendal");
+	printf("%s\n", rbs);
 }
+
+int main(int argc, char **argv)
+{
+
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_hello_opendal),

Review Comment:
   I still don't see the necessity of introducing cmocka. Should we consider removing it now?



-- 
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