Shouldn't one always check a pointer is not null and aligned before dereferencing

[philomathic_life]

I realize this may be an "extreme" example, but nix::errno::Errno::last_raw "blindly" dereferences the pointer without first ensuring it is not null and aligned:

    pub fn last_raw() -> i32 {
        unsafe { *errno_location() }
    }

While I realize something would seem to be very messed up for __errno_location (and other platform-equivalent functions) to return null or a non-aligned pointer, wouldn't it still be better to verify the conditions are true and then panic if not?

For example, my code looks something like this:

    pub fn last() -> Self {
        #[cfg(any(target_os = "netbsd", target_os = "openbsd"))]
        // SAFETY:
        // Supposed to be a thread-local variable, but there is no
        // "check" to ensure this.
        let ptr = unsafe { c::__errno() };
        #[cfg(any(target_os = "freebsd", target_vendor = "apple"))]
        // SAFETY:
        // Supposed to be a thread-local variable, but there is no
        // "check" to ensure this.
        let ptr = unsafe { c::__error() };
        #[cfg(any(target_os = "dragonfly", target_os = "linux"))]
        // SAFETY:
        // Supposed to be a thread-local variable, but there is no
        // "check" to ensure this.
        let ptr = unsafe { c::__errno_location() };
        assert!(
            !ptr.is_null() && ptr.is_aligned(),
            "libc errno returned a pointer that was either null or not aligned"
        );
        // SAFETY:
        // Verified above that `ptr` is not null and aligned.
        Self::from_raw(unsafe { *ptr })
    }

I realize one can't verify everything about the FFI call (e.g., one can only assume __errno_location is thread-local), but I don't think that means one should not check for things that it can. Is this a case of being too safe on my part?

[Cerber-Ursi]

I guess this is the case when:

• safety conditions are essentially guaranteed, unless something is very wrong with the setup (and in this case, this would be one of lesser problems);
• they can't be optimized away at all, adding a panicking branch...
• ...to possibly relatively hot code path.

One could add a debug_assert! to such places though, to have panicking branch explicitly only with corresponding flag (i.e., under normal circumstances, only without --release).

[philomathic_life]

Makes sense. I still feel uneasy about it though. Luckily my crate, unlike nix, is a much more specialized crate around privilege separation and reduction; thus I'm not particularly worried about "hot paths". I think I'll keep my code as is then, but perhaps I'll resort to debug_assert in other code if I'm worried about performance. I try to not make a habit of using pointers or dealing with FFI, so this is fortunately not a problem I frequently face.

[crossroads1112]

It's obviously a judgement call to some extent, but dereferencing errno_location() should be equivalent to accessing errno in plain C code (and that function is small enough that one could manually verify this for each supported platform), something that, in order for the implementation to be standard's compliant, should always be valid.

If accessing errno would trigger undefined behavior (by dereferncing an unaligned or NULL pointer) in C code, we're probably at a point where all reasoning about what is happening in your program is out the window anyway.