silver-ymz commented on code in PR #2984:
URL:
https://github.com/apache/incubator-opendal/pull/2984#discussion_r1311249353
##########
bindings/cpp/src/opendal.cpp:
##########
@@ -38,8 +38,16 @@ Operator::Operator(std::string_view scheme,
bool Operator::available() const { return operator_.has_value(); }
std::vector<uint8_t> Operator::read(std::string_view path) {
- auto res = operator_.value()->read(rust::Str(path.data()));
- return std::vector<uint8_t>(res.data(), res.data() + res.size());
+ auto rust_vec = operator_.value()->read(rust::Str(path.data()));
+
+ // Convert rust::Vec<uint8_t> to std::vector<uint8_t>
+ // This cannot use rust vector pointer to init std::vector because
+ // rust::Vec owns the memory and will free it when it goes out of scope.
+ std::vector<uint8_t> res;
+ res.reserve(rust_vec.size());
+ std::copy(rust_vec.cbegin(), rust_vec.cend(), std::back_inserter(res));
Review Comment:
`cxx::CxxVector` can only use `fn push(self: Pin<&mut Self>, value: T)` to
insert a value. This seems still to need copying in rust side.
I researched other storage library's API.
GCS returns
[gcs::ObjectReadStream](https://cloud.google.com/cpp/docs/reference/storage/latest/classgoogle_1_1cloud_1_1storage_1_1Client#classgoogle_1_1cloud_1_1storage_1_1Client_1a8b0575846b68475f6b1f7ef950b49323)
which derived from `std::istream`.
AWS s3 returns
[Aws::IOStream](https://sdk.amazonaws.com/cpp/api/LATEST/aws-cpp-sdk-core/html/namespace_aws.html#a7fceacd06044b767f90ef1ae53f498d8)
which is just alias to `std::iostream`.
So I think it a better solution to return a self-defined stream class which
holds `rust::Vec`.
--
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]
