Skip to content

Commit

Permalink
Add cfg attribute for advanced seekable impls and make seek table cre…
Browse files Browse the repository at this point in the history
…ation safe (#328)

* Fix missing cfg attribute on AdvancedSeekable impls

The missing cfg macros on the impls caused a compilation error when
compiling with feature `seekable` but without `std`

* Make seek table creation safe

The `SeekTable::try_from_seekable()` fn segfaulted when called with an
uninitialized seekable in zstd versions prior to v1.5.7. This is fixed
with v1.5.7, so the fn is safe to use now.
  • Loading branch information
rorosen authored Feb 21, 2025
1 parent 3084ef8 commit ea2ff60
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
10 changes: 3 additions & 7 deletions zstd-safe/src/seekable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,9 @@ pub struct AdvancedSeekable<'a, F> {
src: *mut F,
}

#[cfg(feature = "std")]
unsafe impl<F> Send for AdvancedSeekable<'_, F> where F: Send {}
#[cfg(feature = "std")]
unsafe impl<F> Sync for AdvancedSeekable<'_, F> where F: Sync {}

#[cfg(feature = "std")]
Expand Down Expand Up @@ -597,17 +599,11 @@ unsafe impl Send for SeekTable {}
unsafe impl Sync for SeekTable {}

impl SeekTable {
// `ZSTD_seekTable_create_fromSeekable` causes a segmentation fault when called with an
// uninitialized Seekable. This function is safe once the issue is resolved and released
// upstream.
// See https://github.com/facebook/zstd/issues/4200 and https://github.com/facebook/zstd/pull/4201
/// Try to create a `SeekTable` from a `Seekable`.
///
/// Memory constrained use cases that manage multiple archives benefit from retaining
/// multiple archive seek tables without retaining a `Seekable` instance for each.
///
/// May cause a segmentation fault when called wih an uninitialized `Seekable`.
pub unsafe fn try_from_seekable<'a>(
pub fn try_from_seekable<'a>(
value: &Seekable<'a>,
) -> Result<Self, SeekTableCreateError> {
// Safety: Just FFI
Expand Down
7 changes: 6 additions & 1 deletion zstd-safe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,19 @@ fn test_seekable_seek_table() {

let seekable_archive = new_seekable_archive(INPUT);
let mut seekable = Seekable::create();

// Assert that creating a SeekTable from an uninitialized seekable errors.
// This led to segfaults with zstd versions prior v1.5.7
assert!(SeekTable::try_from_seekable(&seekable).is_err());

seekable
.init_buff(&seekable_archive)
.map_err(zstd_safe::get_error_name)
.unwrap();

// Try to create a seek table from the seekable
let seek_table =
unsafe { SeekTable::try_from_seekable(&seekable).unwrap() };
{ SeekTable::try_from_seekable(&seekable).unwrap() };

// Seekable and seek table should return the same results
assert_eq!(seekable.num_frames(), seek_table.num_frames());
Expand Down

0 comments on commit ea2ff60

Please sign in to comment.