Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Do not check offsets or utf8 validity in ffi (#505) #510

Merged
merged 1 commit into from
Oct 9, 2021

Conversation

NilsBarlaug
Copy link
Contributor

@NilsBarlaug NilsBarlaug commented Oct 8, 2021

As declared in the spec, the purpose of FFI is to be a very low cost API. The method is already intrinsically unsafe because the data must already follow the spec.

Changed Utf8Array::try_from_ffi and BinaryArray::try_from_ffi to use from_data_unchecked instead of from_data.

Change `Utf8Array::try_from_ffi` and `BinaryArray::try_from_ffi` to
use `from_data_unchecked` instead of `from_data`.
@codecov
Copy link

codecov bot commented Oct 8, 2021

Codecov Report

Merging #510 (5ef1920) into main (8a7f1d7) will increase coverage by 0.00%.
The diff coverage is 87.50%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #510   +/-   ##
=======================================
  Coverage   80.17%   80.18%           
=======================================
  Files         381      381           
  Lines       23659    23665    +6     
=======================================
+ Hits        18969    18975    +6     
  Misses       4690     4690           
Impacted Files Coverage Δ
src/array/binary/mod.rs 78.75% <83.33%> (+0.37%) ⬆️
src/array/binary/ffi.rs 84.61% <100.00%> (ø)
src/array/utf8/ffi.rs 77.77% <100.00%> (ø)
src/io/ipc/write/common.rs 76.28% <0.00%> (+0.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a7f1d7...5ef1920. Read the comment docs.

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Oct 9, 2021
@jorgecarleitao jorgecarleitao merged commit f7b343e into jorgecarleitao:main Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants