Skip to content

Commit

Permalink
[rust png] Delete incorrect memory safety comments.
Browse files Browse the repository at this point in the history
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
rust-lang/unsafe-code-guidelines#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 <lukasza@google.com>
Commit-Queue: Daniel Dilan <danieldilan@google.com>
Reviewed-by: Daniel Dilan <danieldilan@google.com>
  • Loading branch information
anforowicz authored and SkCQ committed Oct 1, 2024
1 parent f7cb94e commit 7c61f88
Showing 1 changed file with 0 additions and 16 deletions.
16 changes: 0 additions & 16 deletions experimental/rust_png/ffi/FFI.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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],
Expand Down

0 comments on commit 7c61f88

Please sign in to comment.