From 7c61f88422487b2a8efd180435490d57316472bb Mon Sep 17 00:00:00 2001 From: Lukasz Anforowicz Date: Tue, 1 Oct 2024 17:46:22 +0000 Subject: [PATCH] [rust png] Delete incorrect memory safety comments. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the past I have assumed that the mere **existence** of a `&mut` reference to uninitialized memory results in instant Undefined Behavior (UB), even if there are no explicit reads in the code. This scenario has been recently discussed in the internal chatroom about `unsafe` Rust code (see https://chat.google.com/room/AAAAhLsgrQ4/Fx2naiaXbeU) where https://github.com/rust-lang/unsafe-code-guidelines/issues/346 was linked and where it seems that the consensus is to **not** treat `&mut uninit` as immediate UB. On one hand the discussions are still ongoing, but OTOH I don't want to make/spread safety notes that may very well be incorrect and overly conservative. So, for now, let me delete the related safety comments from `FFI.rs`. Bug: chromium:356884491 Change-Id: Ica15532493dc0c35b12332df04306fe87be10d3e Reviewed-on: https://skia-review.googlesource.com/c/skia/+/904956 Auto-Submit: Łukasz Anforowicz Commit-Queue: Daniel Dilan Reviewed-by: Daniel Dilan --- experimental/rust_png/ffi/FFI.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/experimental/rust_png/ffi/FFI.rs b/experimental/rust_png/ffi/FFI.rs index 2f9c2b3586f9..c61c7caae925 100644 --- a/experimental/rust_png/ffi/FFI.rs +++ b/experimental/rust_png/ffi/FFI.rs @@ -261,10 +261,6 @@ impl Reader { /// If the decoded PNG image contained a `cHRM` chunk then `try_get_chrm` /// returns `true` and populates the out parameters (`wx`, `wy`, `rx`, /// etc.). Otherwise, returns `false`. - /// - /// C++/FFI safety: The caller has to guarantee that all the outputs / - /// `&mut` values have been initialized (unlike in C++, where such - /// guarantees are typically not needed for output parameters). fn try_get_chrm( &self, wx: &mut f32, @@ -296,10 +292,6 @@ impl Reader { /// If the decoded PNG image contained a `gAMA` chunk then `try_get_gama` /// returns `true` and populates the `gamma` out parameter. Otherwise, /// returns `false`. - /// - /// C++/FFI safety: The caller has to guarantee that all the outputs / - /// `&mut` values have been initialized (unlike in C++, where such - /// guarantees are typically not needed for output parameters). fn try_get_gama(&self, gamma: &mut f32) -> bool { match self.reader.info().gama_chunk.as_ref() { None => false, @@ -360,10 +352,6 @@ impl Reader { /// Returns `png::FrameControl` information. /// /// Panics if no `fcTL` chunk hasn't been parsed yet. - /// - /// C++/FFI safety: The caller has to guarantee that all the outputs / - /// `&mut` values have been initialized (unlike in C++, where such - /// guarantees are typically not needed for output parameters). fn get_fctl_info( self: &Reader, width: &mut u32, @@ -424,10 +412,6 @@ impl Reader { /// Expands the last decoded interlaced row - see /// https://docs.rs/png/latest/png/fn.expand_interlaced_row - /// - /// C++/FFI safety: The caller has to guarantee that `img` doesn't - /// contain uninitialized memory (this is a bit different from C++, - /// where a write-only access may not need such guarantees). fn expand_last_interlaced_row( &self, img: &mut [u8],