Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(binding/c): add is_exist to operator #1892

Merged
merged 1 commit into from
Apr 11, 2023
Merged

Conversation

xyjixyjixyji
Copy link
Contributor

@xyjixyjixyji xyjixyjixyji commented Apr 10, 2023

  • Add is_exist operation to check whether the path exists, which is required in BDD tests.
    • The test for is_exist will be added when we support the Fs backend.

Copy link
Member

@suyanhanx suyanhanx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want binding to be like a native library. We may shouldn't add primitive types like Vec.

@xyjixyjixyji
Copy link
Contributor Author

We want binding to be like a native library. We may shouldn't add primitive types like Vec.

This only expose a pointer to vector. And all APIs are exposed by us.

If we do not expose types like Vec, and only use data+len way, we are going to face following problems.

  • The biggest problem is that the opendal_bytes will be extremely constraint, meaning that the user cannot operate further on this contiguous memory space, the only operation on opendal_bytes will be read and truncate.
  • We can not utilize the Rust optimization on Vec.

bindings/c/include/opendal.h Outdated Show resolved Hide resolved
bindings/c/include/opendal.h Outdated Show resolved Hide resolved
bindings/c/include/opendal.h Outdated Show resolved Hide resolved
bindings/c/include/opendal.h Outdated Show resolved Hide resolved
bindings/c/src/lib.rs Show resolved Hide resolved
bindings/c/tests/basicio.c Outdated Show resolved Hide resolved
@Xuanwo

This comment was marked as resolved.

@xyjixyjixyji
Copy link
Contributor Author

I will change the naming of is_exist and revert the changing on opendal_byte since it is not needed for us.

@Xuanwo
Copy link
Member

Xuanwo commented Apr 10, 2023

I will change the naming of is_exist and revert the changing on opendal_byte since it is not needed for us.

Thank you very much for your effort and patience. I learned a lot about C programming!

* Add is_exist operation
* Update some rustdocs

Signed-off-by: Ji-Xinyou <[email protected]>
@xyjixyjixyji xyjixyjixyji changed the title feat(binding/c): add is_exist and refactor the opendal_bytes feat(binding/c): add is_exist to operator Apr 10, 2023
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks a lot!

@Xuanwo Xuanwo merged commit 4bd89ef into apache:main Apr 11, 2023
wcy-fdu pushed a commit to wcy-fdu/opendal that referenced this pull request Apr 12, 2023
feat(bindings/c): add is_exist operation on operator

* Add is_exist operation
* Update some rustdocs

Signed-off-by: Ji-Xinyou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants