silver-ymz commented on code in PR #2329:
URL: 
https://github.com/apache/incubator-opendal/pull/2329#discussion_r1229717700


##########
bindings/c/include/opendal.h:
##########
@@ -311,6 +338,39 @@ bool opendal_metadata_is_file(const opendal_metadata 
*self);
  */
 bool opendal_metadata_is_dir(const opendal_metadata *self);
 
+/*
+ Construct a heap-allocated opendal_operator_options
+ */
+opendal_operator_options opendal_operator_options_new(void);
+
+/*
+ Set a Key-Value pair inside opendal_operator_options
+
+ # Safety
+
+ This function is unsafe because it dereferences and casts the raw pointers
+ Make sure the pointer of `key` and `value` point to a valid string.
+
+ # Example
+
+ ```C
+ opendal_operator_options options = opendal_operator_options_new();
+ opendal_operator_options_set(&options, "root", "/myroot");
+
+ // .. use your opendal_operator_options
+
+ opendal_operator_options_free(options);
+ ```

Review Comment:
   In this line, it passes wrong type to `opendal_operator_options_free`. It 
should be `opendal_operator_options_free(&options);` 
   
   Moreover, `opendal_operator_options_new` returns a value, but `set` and 
`free` requires a pointer. It seems easy to make mistakes here. Maybe it's 
better to unify the types.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to