Skip to content

Commit

Permalink
Fix unsound problem in Row::row_unchecked (#5)
Browse files Browse the repository at this point in the history
Hi, thanks for your folk of arrow and best wishes first.

We have found a simple unsound problem in `::compute::sort::row::Rows`.
The function
[`Rows::row_unchecked`](https://github.com/rerun-io/re_arrow2/blob/33a32000001df800e4840d92c33b03e7007311e1/src/compute/sort/row/mod.rs#L272)
is wrongly marked as safe, which would confuse the boundary between safe
and unsafe Rust.

Since the repository of arrow2 have been archived, we cannot create any
issue report. So we directly open this PR to mark the function
`row_unchecked` as 'unsafe' and add a `debug_assert!` macro for safety
consideration.
  • Loading branch information
safe4u authored Jul 9, 2024
1 parent 6c2e724 commit 65da4f3
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/compute/sort/row/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ pub struct Rows {

impl Rows {
/// Get a reference to a certain row.
///
/// # Panics
///
/// Panics if `row` is out of bounds
pub fn row(&self, row: usize) -> Row<'_> {
let end = self.offsets[row + 1];
let start = self.offsets[row];
Expand All @@ -269,7 +273,12 @@ impl Rows {
}

/// Get a reference to a certain row but not check the bounds.
pub fn row_unchecked(&self, row: usize) -> Row<'_> {
///
/// # Safety
///
/// The caller must ensure that `row` is within bounds
pub unsafe fn row_unchecked(&self, row: usize) -> Row<'_> {
debug_assert!(row < self.len(), "out-of-bounds access");
let data = unsafe {
let end = *self.offsets.get_unchecked(row + 1);
let start = *self.offsets.get_unchecked(row);
Expand Down Expand Up @@ -319,7 +328,7 @@ impl<'a> Iterator for RowsIter<'a> {
#[inline]
fn next(&mut self) -> Option<Self::Item> {
if self.start < self.end {
let row = self.rows.row_unchecked(self.start);
let row = unsafe { self.rows.row_unchecked(self.start) };
self.start += 1;
Some(row)
} else {
Expand Down

0 comments on commit 65da4f3

Please sign in to comment.