remove unsafe from skrifa... (#824)

* remove unsafe from skrifa...

and make both read-fonts and skrifa #![forbid(unsafe_code)]

* fix no_std

* remove unnecessary safety comment

* just unwrap from_utf8() in PString::read()

since we already check if is_ascii()

font-types/src/fixed.rs

@@ -7,7 +7,7 @@ macro_rules! fixed_impl {
     ($name:ident, $bits:literal, $fract_bits:literal, $ty:ty) => {
         #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
         #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-        #[cfg_attr(feature = "bytemuck", derive(bytemuck::AnyBitPattern))]
+        #[cfg_attr(feature = "bytemuck", derive(bytemuck::AnyBitPattern, bytemuck::NoUninit))]
         #[repr(transparent)]
         #[doc = concat!(stringify!($bits), "-bit signed fixed point number with ", stringify!($fract_bits), " bits of fraction." )]
         pub struct $name($ty);

font-types/src/point.rs

@@ -12,6 +12,26 @@ pub struct Point<T> {
     pub y: T,
 }
 
+/// SAFETY:
+/// This trait has four preconditions:
+///
+/// 1. All fields in the struct must implement `NoUninit`
+/// 2. The struct must be `#[repr(C)]` or `#[repr(transparent)]`
+/// 3. The struct must not contain any padding bytes
+/// 4. The struct must contain no generic parameters
+///
+/// We satisfy the first and second preconditions trivially. The third
+/// condition is satisfied because the struct is repr(C) and contains
+/// two fields of the same type which guarantees no padding.
+///
+/// The fourth condition is obviously not satisfied which is what
+/// requires implementing this trait manually rather than deriving
+/// it. This condition only exists because the bytemuck derive
+/// macro cannot guarantee the first three conditions in a type
+/// with generic parameters.
+#[cfg(feature = "bytemuck")]
+unsafe impl<T> bytemuck::NoUninit for Point<T> where T: bytemuck::NoUninit {}
+
 impl<T> Point<T> {
     /// Creates a new point with the given x and y coordinates.
     #[inline(always)]

read-fonts/src/lib.rs

@@ -56,6 +56,7 @@
 //! [NameString]: tables::name::NameString
 //! [table-directory]: https://learn.microsoft.com/en-us/typography/opentype/spec/otff#table-directory
 
+#![forbid(unsafe_code)]
 #![deny(rustdoc::broken_intra_doc_links)]
 #![cfg_attr(not(feature = "std"), no_std)]
 

read-fonts/src/tables/glyf.rs

@@ -35,7 +35,9 @@ impl PointMarker {
 /// Some properties, such as on- and off-curve flags are intrinsic to the point
 /// itself. Others, designated as markers are set and cleared while an outline
 /// is being transformed during variation application and hinting.
-#[derive(Copy, Clone, PartialEq, Eq, Default, Debug)]
+#[derive(
+    Copy, Clone, PartialEq, Eq, Default, Debug, bytemuck::AnyBitPattern, bytemuck::NoUninit,
+)]
 #[repr(transparent)]
 pub struct PointFlags(u8);
 

read-fonts/src/tables/post.rs

@@ -72,10 +72,9 @@ impl<'a> FontRead<'a> for PString<'a> {
             .as_bytes()
             .get(1..len as usize + 1)
             .ok_or(ReadError::OutOfBounds)?;
+
         if pstring.is_ascii() {
-            // SAFETY: `pstring` must be valid UTF-8, which is known to be the
-            // case since ASCII is a subset of UTF-8 and `is_ascii()` is true.
-            Ok(PString(unsafe { std::str::from_utf8_unchecked(pstring) }))
+            Ok(PString(std::str::from_utf8(pstring).unwrap()))
         } else {
             //FIXME not really sure how we want to handle this?
             Err(ReadError::MalformedData("Must be valid ascii"))

skrifa/Cargo.toml

@@ -17,6 +17,7 @@ libm = ["dep:core_maths"]
 [dependencies]
 read-fonts = { version = "0.17.0", path = "../read-fonts", default-features = false }
 core_maths = { version = "0.1", optional = true }
+bytemuck = "1.14.3"
 
 [dev-dependencies]
 font-test-data = { path = "../font-test-data" }

skrifa/src/lib.rs

@@ -11,6 +11,7 @@
 //! See the [readme](https://github.com/googlefonts/fontations/blob/main/skrifa/README.md)
 //! for additional details.
 
+#![forbid(unsafe_code)]
 #![cfg_attr(not(any(test, feature = "std")), no_std)]
 
 #[cfg(not(any(feature = "libm", feature = "std")))]

skrifa/src/outline/glyf/memory.rs

@@ -101,26 +101,14 @@ impl<'a> OutlineMemory<'a> {
     }
 }
 
-/// Trait that defines which types may be used as element types for
-/// `alloc_slice`.
-///
-/// # Safety
-/// This must only be implemented for types that contain no internal padding
-/// and for which all bit patterns are valid.
-unsafe trait TransmuteElement: Sized + Copy {}
-
-unsafe impl TransmuteElement for u16 {}
-unsafe impl TransmuteElement for i32 {}
-unsafe impl TransmuteElement for Fixed {}
-unsafe impl TransmuteElement for F26Dot6 {}
-unsafe impl TransmuteElement for PointFlags {}
-unsafe impl<T> TransmuteElement for Point<T> where T: TransmuteElement {}
-
 /// Allocates a mutable slice of `T` of the given length from the specified
 /// buffer.
 ///
 /// Returns the allocated slice and the remainder of the buffer.
-fn alloc_slice<T: TransmuteElement>(buf: &mut [u8], len: usize) -> Option<(&mut [T], &mut [u8])> {
+fn alloc_slice<T>(buf: &mut [u8], len: usize) -> Option<(&mut [T], &mut [u8])>
+where
+    T: bytemuck::AnyBitPattern + bytemuck::NoUninit,
+{
     if len == 0 {
         return Some((Default::default(), buf));
     }
@@ -136,11 +124,8 @@ fn alloc_slice<T: TransmuteElement>(buf: &mut [u8], len: usize) -> Option<(&mut
         return None;
     }
     let (slice_buf, rest) = buf.split_at_mut(len_in_bytes);
-    // SAFETY: Alignment and size requirements have been checked in 1) and
-    // 2) above, respectively.
-    // Element types are limited by the `TransmuteElement` trait which
-    // defines requirements for transmutation and is private to this module.
-    let slice = unsafe { std::slice::from_raw_parts_mut(slice_buf.as_mut_ptr() as *mut T, len) };
+    // Bytemuck handles all safety guarantees here.
+    let slice = bytemuck::try_cast_slice_mut(slice_buf).ok()?;
     Some((slice, rest))
 }