From ea2ff60f20dface6735bf7b99b15db253f8c5f0f Mon Sep 17 00:00:00 2001 From: rorosen <76747196+rorosen@users.noreply.github.com> Date: Fri, 21 Feb 2025 13:34:24 +0100 Subject: [PATCH] Add cfg attribute for advanced seekable impls and make seek table creation 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. --- zstd-safe/src/seekable.rs | 10 +++------- zstd-safe/src/tests.rs | 7 ++++++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/zstd-safe/src/seekable.rs b/zstd-safe/src/seekable.rs index 47c43026..276b59bc 100644 --- a/zstd-safe/src/seekable.rs +++ b/zstd-safe/src/seekable.rs @@ -455,7 +455,9 @@ pub struct AdvancedSeekable<'a, F> { src: *mut F, } +#[cfg(feature = "std")] unsafe impl Send for AdvancedSeekable<'_, F> where F: Send {} +#[cfg(feature = "std")] unsafe impl Sync for AdvancedSeekable<'_, F> where F: Sync {} #[cfg(feature = "std")] @@ -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 { // Safety: Just FFI diff --git a/zstd-safe/src/tests.rs b/zstd-safe/src/tests.rs index 98695331..986da3f0 100644 --- a/zstd-safe/src/tests.rs +++ b/zstd-safe/src/tests.rs @@ -209,6 +209,11 @@ 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) @@ -216,7 +221,7 @@ fn test_seekable_seek_table() { // 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());