You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "jiaoew1991 (via GitHub)" <gi...@apache.org> on 2023/10/11 21:56:04 UTC
[PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
jiaoew1991 opened a new pull request, #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259
issue: https://github.com/apache/incubator-opendal/issues/3257
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1759976948
> should I improve this pull request, or should I open a new one? 🤔
Both LGTM, I think you can work on this PR directly.
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1758877660
> Hmm, so we setup the reader (buffer size, path, etc.) on the first function, and then do the read that follows the configuration?
The input write buffer size should be passed in every `read` call the same as in this PR.
I am concerned about the API semantics. We will not offer `opendal_reader_buffer_read()` because it implies that we will have a `opendal_reader_read()`.
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1758854605
To be more specific, we could have two functions to reach this approach
1. `opendal_reader_new()` to construct a new Reader
2. `opendal_reader_buffer_read()` to read through a Reader
For the first function, you could refer to the `opendal_operator_new()`. For the second part, your implementation in this PR will not change much.
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357266507
##########
bindings/c/include/opendal.h:
##########
@@ -524,6 +524,53 @@ enum opendal_code opendal_operator_blocking_write(const struct opendal_operator_
struct opendal_result_read opendal_operator_blocking_read(const struct opendal_operator_ptr *ptr,
const char *path);
+/**
+ * \brief Blockingly read the data from `path`.
+ *
+ * Read the data out from `path` blockingly by operator, returns
+ * an opendal_result_read with error code.
+ *
+ * @param ptr The opendal_operator_ptr created previously
+ * @param path The path you want to read the data out
+ * @param buffer The buffer you want to read the data into
+ * @param buffer_len The length of the buffer
+ * @see opendal_operator_ptr
+ * @see opendal_result_read
+ * @see opendal_code
+ * @return Returns opendal_code
+ *
+ * \note If the read operation succeeds, the returned opendal_bytes is newly allocated on heap.
+ * After your usage of that, please call opendal_bytes_free() to free the space.
+ *
+ * # Example
+ *
+ * Following is an example
+ * ```C
+ * // ... you have write "Hello, World!" to path "/testpath"
+ *
+ * int length = 13;
+ * unsigned char buffer[length];
+ * opendal_code r = opendal_operator_blocking_read_with_buffer(ptr, "testpath", buffer, length);
+ * assert(r == OPENDAL_OK);
+ * // assert buffer == "Hello, World!"
+ *
+ * ```
+ *
+ * # Safety
+ *
+ * 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.
+ *
+ * # Panic
+ *
+ * * If the `path` points to NULL, this function panics, i.e. exits with information
+ */
+enum opendal_code opendal_operator_blocking_read_with_buffer(const struct opendal_operator_ptr *ptr,
Review Comment:
changed it.
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1759915622
Hi @Xuanwo @Ji-Xinyou , should I improve this pull request, or should I open a new one? 🤔
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou closed pull request #3259: feat(binding/c): Add blocking_reader for C binding
URL: https://github.com/apache/incubator-opendal/pull/3259
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357735343
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
> You could have a `opendal_result_reader_read` with an `opendal_code` and a `size`.
By the way, do we need to allocate on the success case after migrating to `opendal_error`?
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357721644
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
We should return error here, can be done after https://github.com/apache/incubator-opendal/pull/3250
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1355933321
##########
bindings/c/include/opendal.h:
##########
@@ -524,6 +524,53 @@ enum opendal_code opendal_operator_blocking_write(const struct opendal_operator_
struct opendal_result_read opendal_operator_blocking_read(const struct opendal_operator_ptr *ptr,
const char *path);
+/**
+ * \brief Blockingly read the data from `path`.
+ *
+ * Read the data out from `path` blockingly by operator, returns
+ * an opendal_result_read with error code.
+ *
+ * @param ptr The opendal_operator_ptr created previously
+ * @param path The path you want to read the data out
+ * @param buffer The buffer you want to read the data into
+ * @param buffer_len The length of the buffer
+ * @see opendal_operator_ptr
+ * @see opendal_result_read
+ * @see opendal_code
+ * @return Returns opendal_code
+ *
+ * \note If the read operation succeeds, the returned opendal_bytes is newly allocated on heap.
+ * After your usage of that, please call opendal_bytes_free() to free the space.
+ *
+ * # Example
+ *
+ * Following is an example
+ * ```C
+ * // ... you have write "Hello, World!" to path "/testpath"
+ *
+ * int length = 13;
+ * unsigned char buffer[length];
+ * opendal_code r = opendal_operator_blocking_read_with_buffer(ptr, "testpath", buffer, length);
+ * assert(r == OPENDAL_OK);
+ * // assert buffer == "Hello, World!"
+ *
+ * ```
+ *
+ * # Safety
+ *
+ * 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.
+ *
+ * # Panic
+ *
+ * * If the `path` points to NULL, this function panics, i.e. exits with information
+ */
+enum opendal_code opendal_operator_blocking_read_with_buffer(const struct opendal_operator_ptr *ptr,
Review Comment:
I dislike the `read_with_buffer` API as it may conflict with `read_with("path").buffer(4096)`.
What if we expose `opendal_operator_blocking_reader` that returns an `opendal_reader_ptr` instead? Users can directly call `read`/`seek` on the reader. I think this API will provide more power and control to users over the data reading logic.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357735343
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
> You could have a `opendal_result_reader_read` with an `opendal_code` and a `size`.
By the way, do we need to allocate on the success case after migrating to `opendal_error`? The cost is the same with returning a `size`?
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357734704
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
You could have a `opendal_result_reader_read` with an `opendal_code` and a `size`.
After the error message pr is merged, we will change `opendal_code` to `opendal_error`
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1762591163
> I found an issue that our naming for structs and functions is inconsistent. The word `blocking` shown up randomly.
>
> cc @Ji-Xinyou, how about removing all `blocking` from our API for now? We can add `async` version in the future.
Sure, that could be the default case since blocking is the most common.
When we add async in the future, we can add async keyword in the API.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1761415052
Hi, @jiaoew1991, https://github.com/apache/incubator-opendal/pull/3250 is about to merge. Maybe you can work upon it directly?
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1355944141
##########
bindings/c/include/opendal.h:
##########
@@ -524,6 +524,53 @@ enum opendal_code opendal_operator_blocking_write(const struct opendal_operator_
struct opendal_result_read opendal_operator_blocking_read(const struct opendal_operator_ptr *ptr,
const char *path);
+/**
+ * \brief Blockingly read the data from `path`.
+ *
+ * Read the data out from `path` blockingly by operator, returns
+ * an opendal_result_read with error code.
+ *
+ * @param ptr The opendal_operator_ptr created previously
+ * @param path The path you want to read the data out
+ * @param buffer The buffer you want to read the data into
+ * @param buffer_len The length of the buffer
+ * @see opendal_operator_ptr
+ * @see opendal_result_read
+ * @see opendal_code
+ * @return Returns opendal_code
+ *
+ * \note If the read operation succeeds, the returned opendal_bytes is newly allocated on heap.
+ * After your usage of that, please call opendal_bytes_free() to free the space.
+ *
+ * # Example
+ *
+ * Following is an example
+ * ```C
+ * // ... you have write "Hello, World!" to path "/testpath"
+ *
+ * int length = 13;
+ * unsigned char buffer[length];
+ * opendal_code r = opendal_operator_blocking_read_with_buffer(ptr, "testpath", buffer, length);
+ * assert(r == OPENDAL_OK);
+ * // assert buffer == "Hello, World!"
+ *
+ * ```
+ *
+ * # Safety
+ *
+ * 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.
+ *
+ * # Panic
+ *
+ * * If the `path` points to NULL, this function panics, i.e. exits with information
+ */
+enum opendal_code opendal_operator_blocking_read_with_buffer(const struct opendal_operator_ptr *ptr,
Review Comment:
> I dislike the `read_with_buffer` API as it may conflict with `read_with("path").buffer(4096)`.
>
> What if we expose `opendal_operator_blocking_reader` that returns an `opendal_reader_ptr` instead? Users can directly call `read`/`seek` on the reader. I think this API will provide more power and control to users over the data reading logic.
I agree, we should first have a opendal_reader, then use that reader to do the buffered read.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1762317979
I closed and reopened this PR to rerun the CI.
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1758872533
> 2\. `opendal_reader_buffer_read()` to read through a Reader
Negative. We should only have `opendal_reader_read()` that accepts an address passed by the C side.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1762558075
I found an issue that our naming for structs and functions is inconsistent. The word `blocking` shown up randomly.
cc @Ji-Xinyou, how about removing all `blocking` from our API for now? We can add `async` version in the future.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357733740
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
> I think it would be better to return the size of the buffer being read. 🤔
Yes, we should return the value of `written_size` upon success and an error message upon failure.
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
> I think it would be better to return the size of the buffer being read. 🤔
Yes, we should return the value of `written_size` upon success and an error message upon failure.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1760736521
All other parts look LGTM, except for the return value of reader read. I mean it is always good to have as much error information as possible 🤣
--
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
Re: [PR] Add blocking_read_with_buffer for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1758873973
> > 2. `opendal_reader_buffer_read()` to read through a Reader
>
> Negative. We should only have `opendal_reader_read()` that accepts an address passed by the C side.
Hmm, so we setup the reader (buffer size, path, etc.) on the first function, and then do the read that follows the configuration?
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1761796172
@Xuanwo Updated it, but most of the CI jobs are cancelled 😅
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "jiaoew1991 (via GitHub)" <gi...@apache.org>.
jiaoew1991 commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357733112
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
At first, it was returning "opendal_error", but upon further consideration, I think it would be better to return the size of the buffer being read. 🤔
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
At first, it was returning "opendal_error", but upon further consideration, I think it would be better to return the size of the buffer being read. 🤔
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
At first, it was returning "opendal_code", but upon further consideration, I think it would be better to return the size of the buffer being read. 🤔
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Ji-Xinyou (via GitHub)" <gi...@apache.org>.
Ji-Xinyou commented on code in PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#discussion_r1357735773
##########
bindings/c/include/opendal.h:
##########
@@ -862,6 +928,10 @@ char *opendal_list_entry_name(const struct opendal_list_entry *self);
*/
void opendal_list_entry_free(struct opendal_list_entry *ptr);
+intptr_t opendal_reader_read(const struct opendal_reader *self, uint8_t *buf, uintptr_t len);
Review Comment:
I don't think we need to allocate. Because on success, the error is a nullptr and all other values are on stack.
--
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
Re: [PR] feat(binding/c): Add blocking_reader for C binding [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3259:
URL: https://github.com/apache/incubator-opendal/pull/3259#issuecomment-1762591965
> Sure, that could be the default case since blocking is the most common.
>
> When we add async in the future, we can add async keyword in the API.
Cool, would you like to submit a PR after this one merged?
--
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