You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/12/23 15:12:03 UTC
[GitHub] [arrow] jorgecarleitao opened a new pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
jorgecarleitao opened a new pull request #8997:
URL: https://github.com/apache/arrow/pull/8997
Currently, our allocation code is not guaranteeing that the `std::mem::alloc` was successful, by checking for whether the returned pointer was not null. Passing null pointers to buffers is dangerous, specially given that Buffers currently expose them without any checks.
This PR is a series of modifications that removes the possibility of having null pointers:
* Made most of our pointers `NonNull` and panic whenever a null pointer tries to sneak to a buffer (either via FFI or a failed allocation)
* Guard against overflow of a pointer address during allocations (relevant for 32 bit systems)
* remove the possibility of a null pointer to be on `RawPtrBox`, flags `RawPtrBox::new` as `unsafe` and documents the invariants necessary to a sound usage of `RawPtrBox`.
* Made all methods in `memory` expect and output a `NonNull`
All these changes were highly motivated by the code in Rust's `std::alloc`, and how it deals with these edge cases.
The main consequence of these changes is that our buffers no longer hold null pointers, which allow us to implement `Deref<[u8]>` (done in this PR), and treat `Buffer` as very similar to an immutable `Vec<u8>` (and `MutableBuffer` closer to `Vec<u8>`). In this direction, this PR renames a bunch of methods:
* `MutableBuffer::data_mut -> MutableBuffer::as_slice_mut`
* `MutableBuffer::data -> MutableBuffer::as_slice`
* `Buffer::data -> Buffer::as_slice`
* `Buffer::raw_data -> Buffer::as_ptr`
* `RawPtrBox::get -> RawPtrBox::as_ptr`
The rational for these names come from `Vec::as_slice_mut`, `Vec::as_slice`, `Vec::as_ptr` and `NonNull::as_ptr` respectively.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] codecov-io edited a comment on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750369770
# [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=h1) Report
> Merging [#8997](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=desc) (792ed6e) into [master](https://codecov.io/gh/apache/arrow/commit/c0dad80a10cd442b4c9c640bba57306bc1ac1f5a?el=desc) (c0dad80) will **increase** coverage by `0.02%`.
> The diff coverage is `95.98%`.
[](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8997 +/- ##
==========================================
+ Coverage 82.64% 82.66% +0.02%
==========================================
Files 200 200
Lines 49719 49778 +59
==========================================
+ Hits 41091 41151 +60
+ Misses 8628 8627 -1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion-testing/src/bin/arrow-json-integration-test.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9pbnRlZ3JhdGlvbi10ZXN0aW5nL3NyYy9iaW4vYXJyb3ctanNvbi1pbnRlZ3JhdGlvbi10ZXN0LnJz) | `0.00% <0.00%> (ø)` | |
| [rust/arrow/src/array/array\_boolean.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYm9vbGVhbi5ycw==) | `86.50% <85.71%> (-0.22%)` | :arrow_down: |
| [rust/arrow/src/bytes.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnl0ZXMucnM=) | `58.33% <87.50%> (+4.27%)` | :arrow_up: |
| [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.27% <92.30%> (+0.02%)` | :arrow_up: |
| [rust/arrow/src/buffer.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnVmZmVyLnJz) | `98.21% <92.72%> (-0.01%)` | :arrow_down: |
| [rust/arrow/src/memory.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvbWVtb3J5LnJz) | `98.14% <96.55%> (-1.86%)` | :arrow_down: |
| [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.73% <100.00%> (ø)` | |
| [rust/arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfbGlzdC5ycw==) | `93.10% <100.00%> (-0.02%)` | :arrow_down: |
| [rust/arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfcHJpbWl0aXZlLnJz) | `91.99% <100.00%> (-0.29%)` | :arrow_down: |
| [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.37% <100.00%> (ø)` | |
| ... and [38 more](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=footer). Last update [c0dad80...4920984](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-751466829
@alamb , thanks a lot for taking the time to review this. This one is challenging to review.
wrt to the `vec<u8>`, my current hypothesis is that the alignment does improve performance when SIMD is on (as the benches show in that PR). As such, my current direction is to get the best of both worlds: migrate the relevant parts of `std::rawbytes::RawBytes` and `std::alloc` to `memory.rs` (as that code is currently `unstable`) to still allocate aligned, but use the well studied code from rust's std. This is also related to #9016 .
Basically, we have a performance problem in the reallocation code. The following is the result of 4 runs:
```
mutable time: [929.26 us 931.88 us 935.42 us]
mutable prepared time: [1.0682 ms 1.0693 ms 1.0709 ms]
from_slice time: [4.4857 ms 4.5043 ms 4.5247 ms]
from_slice prepared time: [1.4358 ms 1.4406 ms 1.4467 ms]
```
1. start with an empty mutable and grow it.
2. start with a mutable with the correct capacity and grows (i.e. extend without re-allocation).
3. do the same with a `vec<u8>` and at the end of all use `Buffer::from`
4. same as 2 and at the end of all use `Buffer::from`
The fact that there is no difference between 1 and 2 but a 3.5x difference between 3 and 4 shows that we are doing something wrong.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750355748
https://issues.apache.org/jira/browse/ARROW-10692
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jhorstmann commented on a change in pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jhorstmann commented on a change in pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#discussion_r550760971
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -213,13 +222,33 @@ impl<T: AsRef<[u8]>> From<T> for Buffer {
let len = slice.len() * mem::size_of::<u8>();
let capacity = bit_util::round_upto_multiple_of_64(len);
let buffer = memory::allocate_aligned(capacity);
+ // JUSTIFICATION
+ // Benefit
+ // It is often useful to create a buffer from bytes, typically when they are allocated by external sources
+ // Soundness
+ // * The pointers are non-null by construction
+ // * alignment asserted above
+ // Unsoundness
+ // * There is no guarantee that the memory regions do are non-overalling, but `memcpy` requires this.
Review comment:
Since `buffer` is freshly allocated it should not be possible for the memory regions to overlap
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750754475
@vertexclique , would you mind taking a look at this? You are an expert around these. :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on a change in pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#discussion_r549110171
##########
File path: rust/arrow/src/array/raw_pointer.rs
##########
@@ -16,28 +16,37 @@
// under the License.
use crate::memory;
+use std::ptr::NonNull;
+/// This struct is highly `unsafe` and offers the possibility to self-reference a [arrow::Buffer] from [arrow::array::ArrayData].
+/// as a pointer to the beginning of its contents.
pub(super) struct RawPtrBox<T> {
- inner: *const T,
+ ptr: NonNull<T>,
}
impl<T> RawPtrBox<T> {
- pub(super) fn new(inner: *const T) -> Self {
- Self { inner }
+ /// # Safety
+ /// The user must guarantee that:
+ /// * the contents where `ptr` points to are never `moved`. This is guaranteed when they are Pinned.
+ /// * the lifetime of this struct does not outlive the lifetime of `ptr`.
+ /// Failure to fulfill any the above conditions results in undefined behavior.
+ /// # Panic
+ /// This function panics if:
+ /// * `ptr` is null
+ /// * `ptr` is not aligned to a slice of type `T`. This is guaranteed if it was built from a slice of type `T`.
+ pub(super) unsafe fn new(ptr: *const u8) -> Self {
Review comment:
This is a nice change -- to move the alignment assertion into the `RawPtrBox::new` as that now makes the callsites clearer as well as they can't forget to ensure alignment. 👍
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -183,7 +192,7 @@ impl Buffer {
/// in larger chunks and starting at arbitrary bit offsets.
/// Note that both `offset` and `length` are measured in bits.
pub fn bit_chunks(&self, offset: usize, len: usize) -> BitChunks {
- BitChunks::new(&self.data.as_slice()[self.offset..], offset, len)
Review comment:
👍
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -919,24 +962,18 @@ mod tests {
#[test]
fn test_from_raw_parts() {
- let buf = unsafe { Buffer::from_raw_parts(null_mut(), 0, 0) };
Review comment:
why was this test removed? Because it is no longer possible to create a buffer from a null pointer?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] codecov-io edited a comment on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750369770
# [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=h1) Report
> Merging [#8997](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=desc) (23846ec) into [master](https://codecov.io/gh/apache/arrow/commit/a4f7c4a2acda874b3d6eb2eb4c986e7c3267c755?el=desc) (a4f7c4a) will **increase** coverage by `0.00%`.
> The diff coverage is `95.51%`.
[](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8997 +/- ##
=======================================
Coverage 82.87% 82.87%
=======================================
Files 201 201
Lines 49739 49746 +7
=======================================
+ Hits 41220 41228 +8
+ Misses 8519 8518 -1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion-testing/src/bin/arrow-json-integration-test.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9pbnRlZ3JhdGlvbi10ZXN0aW5nL3NyYy9iaW4vYXJyb3ctanNvbi1pbnRlZ3JhdGlvbi10ZXN0LnJz) | `0.00% <0.00%> (ø)` | |
| [rust/arrow/src/array/array\_boolean.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYm9vbGVhbi5ycw==) | `86.50% <85.71%> (-0.22%)` | :arrow_down: |
| [rust/arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfcHJpbWl0aXZlLnJz) | `91.64% <85.71%> (-0.05%)` | :arrow_down: |
| [rust/arrow/src/bytes.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnl0ZXMucnM=) | `58.33% <87.50%> (+4.27%)` | :arrow_up: |
| [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.27% <92.30%> (+0.02%)` | :arrow_up: |
| [rust/arrow/src/buffer.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnVmZmVyLnJz) | `98.21% <92.72%> (-0.01%)` | :arrow_down: |
| [rust/arrow/src/memory.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvbWVtb3J5LnJz) | `98.14% <96.55%> (-1.86%)` | :arrow_down: |
| [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.73% <100.00%> (ø)` | |
| [rust/arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfbGlzdC5ycw==) | `93.10% <100.00%> (-0.02%)` | :arrow_down: |
| [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.37% <100.00%> (ø)` | |
| ... and [28 more](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=footer). Last update [a4f7c4a...23846ec](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#discussion_r549111071
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -919,24 +962,18 @@ mod tests {
#[test]
fn test_from_raw_parts() {
- let buf = unsafe { Buffer::from_raw_parts(null_mut(), 0, 0) };
Review comment:
Exactly :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#discussion_r550860990
##########
File path: rust/arrow/src/buffer.rs
##########
@@ -213,13 +222,33 @@ impl<T: AsRef<[u8]>> From<T> for Buffer {
let len = slice.len() * mem::size_of::<u8>();
let capacity = bit_util::round_upto_multiple_of_64(len);
let buffer = memory::allocate_aligned(capacity);
+ // JUSTIFICATION
+ // Benefit
+ // It is often useful to create a buffer from bytes, typically when they are allocated by external sources
+ // Soundness
+ // * The pointers are non-null by construction
+ // * alignment asserted above
+ // Unsoundness
+ // * There is no guarantee that the memory regions do are non-overalling, but `memcpy` requires this.
Review comment:
Good point. This was copied from a previous version and it slipped. :/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] codecov-io edited a comment on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750369770
# [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=h1) Report
> Merging [#8997](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=desc) (a6531ec) into [master](https://codecov.io/gh/apache/arrow/commit/51672b28e97f19f70de0f0a8800c40ee9bb939d3?el=desc) (51672b2) will **increase** coverage by `0.00%`.
> The diff coverage is `95.51%`.
[](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8997 +/- ##
=======================================
Coverage 82.61% 82.61%
=======================================
Files 202 202
Lines 50048 50055 +7
=======================================
+ Hits 41347 41354 +7
Misses 8701 8701
```
| [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion-testing/src/bin/arrow-json-integration-test.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9pbnRlZ3JhdGlvbi10ZXN0aW5nL3NyYy9iaW4vYXJyb3ctanNvbi1pbnRlZ3JhdGlvbi10ZXN0LnJz) | `0.00% <0.00%> (ø)` | |
| [rust/arrow/src/array/array\_boolean.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYm9vbGVhbi5ycw==) | `86.50% <85.71%> (-0.22%)` | :arrow_down: |
| [rust/arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfcHJpbWl0aXZlLnJz) | `91.64% <85.71%> (-0.05%)` | :arrow_down: |
| [rust/arrow/src/bytes.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnl0ZXMucnM=) | `58.33% <87.50%> (+4.27%)` | :arrow_up: |
| [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.27% <92.30%> (+0.02%)` | :arrow_up: |
| [rust/arrow/src/buffer.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnVmZmVyLnJz) | `98.21% <92.72%> (-0.01%)` | :arrow_down: |
| [rust/arrow/src/memory.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvbWVtb3J5LnJz) | `98.14% <96.55%> (-1.86%)` | :arrow_down: |
| [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.61% <100.00%> (ø)` | |
| [rust/arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfbGlzdC5ycw==) | `93.10% <100.00%> (-0.02%)` | :arrow_down: |
| [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.27% <100.00%> (ø)` | |
| ... and [29 more](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=footer). Last update [51672b2...a6531ec](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-753291331
@Dandandan @alamb @nevi-me @jhorstmann : this was merged and had some changes to the names of public methods of the Buffer and MutableBuffer (they are now similar to the ones in `std::Vec`. Just fyi, as you have been working with them :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] codecov-io edited a comment on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750369770
# [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=h1) Report
> Merging [#8997](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=desc) (f8caced) into [master](https://codecov.io/gh/apache/arrow/commit/51672b28e97f19f70de0f0a8800c40ee9bb939d3?el=desc) (51672b2) will **increase** coverage by `0.00%`.
> The diff coverage is `95.51%`.
[](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8997 +/- ##
=======================================
Coverage 82.61% 82.61%
=======================================
Files 202 202
Lines 50048 50055 +7
=======================================
+ Hits 41347 41355 +8
+ Misses 8701 8700 -1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion-testing/src/bin/arrow-json-integration-test.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9pbnRlZ3JhdGlvbi10ZXN0aW5nL3NyYy9iaW4vYXJyb3ctanNvbi1pbnRlZ3JhdGlvbi10ZXN0LnJz) | `0.00% <0.00%> (ø)` | |
| [rust/arrow/src/array/array\_boolean.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYm9vbGVhbi5ycw==) | `86.50% <85.71%> (-0.22%)` | :arrow_down: |
| [rust/arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfcHJpbWl0aXZlLnJz) | `91.64% <85.71%> (-0.05%)` | :arrow_down: |
| [rust/arrow/src/bytes.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnl0ZXMucnM=) | `58.33% <87.50%> (+4.27%)` | :arrow_up: |
| [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.27% <92.30%> (+0.02%)` | :arrow_up: |
| [rust/arrow/src/buffer.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnVmZmVyLnJz) | `98.21% <92.72%> (-0.01%)` | :arrow_down: |
| [rust/arrow/src/memory.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvbWVtb3J5LnJz) | `98.14% <96.55%> (-1.86%)` | :arrow_down: |
| [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.61% <100.00%> (ø)` | |
| [rust/arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfbGlzdC5ycw==) | `93.10% <100.00%> (-0.02%)` | :arrow_down: |
| [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.27% <100.00%> (ø)` | |
| ... and [28 more](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=footer). Last update [51672b2...f8caced](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao closed pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
jorgecarleitao closed pull request #8997:
URL: https://github.com/apache/arrow/pull/8997
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] vertexclique commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
vertexclique commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-751660529
I am going to check out this code and have a look today @jorgecarleitao
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] codecov-io commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-750369770
# [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=h1) Report
> Merging [#8997](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=desc) (1474516) into [master](https://codecov.io/gh/apache/arrow/commit/c0dad80a10cd442b4c9c640bba57306bc1ac1f5a?el=desc) (c0dad80) will **increase** coverage by `0.02%`.
> The diff coverage is `94.82%`.
[](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8997 +/- ##
==========================================
+ Coverage 82.64% 82.67% +0.02%
==========================================
Files 200 200
Lines 49719 49778 +59
==========================================
+ Hits 41091 41152 +61
+ Misses 8628 8626 -2
```
| [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion-testing/src/bin/arrow-json-integration-test.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9pbnRlZ3JhdGlvbi10ZXN0aW5nL3NyYy9iaW4vYXJyb3ctanNvbi1pbnRlZ3JhdGlvbi10ZXN0LnJz) | `0.00% <0.00%> (ø)` | |
| [rust/arrow/src/array/array\_boolean.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYm9vbGVhbi5ycw==) | `86.50% <85.71%> (-0.22%)` | :arrow_down: |
| [rust/arrow/src/bytes.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnl0ZXMucnM=) | `58.33% <87.50%> (+4.27%)` | :arrow_up: |
| [rust/arrow/src/buffer.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYnVmZmVyLnJz) | `98.21% <91.11%> (-0.01%)` | :arrow_down: |
| [rust/parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9yZWNvcmRfcmVhZGVyLnJz) | `96.27% <92.30%> (+0.02%)` | :arrow_up: |
| [rust/arrow/src/memory.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvbWVtb3J5LnJz) | `98.14% <96.55%> (-1.86%)` | :arrow_down: |
| [rust/arrow/src/array/array\_binary.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfYmluYXJ5LnJz) | `90.73% <100.00%> (ø)` | |
| [rust/arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfbGlzdC5ycw==) | `93.10% <100.00%> (-0.02%)` | :arrow_down: |
| [rust/arrow/src/array/array\_primitive.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfcHJpbWl0aXZlLnJz) | `91.99% <100.00%> (-0.29%)` | :arrow_down: |
| [rust/arrow/src/array/array\_string.rs](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvYXJyYXkvYXJyYXlfc3RyaW5nLnJz) | `90.37% <100.00%> (ø)` | |
| ... and [22 more](https://codecov.io/gh/apache/arrow/pull/8997/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=footer). Last update [c0dad80...792ed6e](https://codecov.io/gh/apache/arrow/pull/8997?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #8997: ARROW-10692: [Rust] Removed undefined behavior derived from null pointers
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #8997:
URL: https://github.com/apache/arrow/pull/8997#issuecomment-752956771
The full set of Rust CI tests did not run on this PR :(
Can you please rebase this PR against [apache/master](https://github.com/apache/arrow) to pick up the changes in https://github.com/apache/arrow/pull/9056 so that they do?
I apologize for the inconvenience.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org