UB in new_texture if newTextureWithDescriptor fails?

[jesdazrez]

I was taking a look at the method and noticed it doesn't check what the newTextureWithDescriptor msg_send! returns.

According to the docs that call can return:

A new MTLTexture instance if the method completed successfully; otherwise nil.

The metal::Texture type wraps a NonNull<_> value (from cargo expand texture):

/// See <https://developer.apple.com/documentation/metal/mtltexture>
pub enum MTLTexture {}
#[repr(transparent)]
pub struct Texture(::foreign_types::export::NonNull<MTLTexture>);

So when that call fails new_texture will put a null ptr in a NonNull which is UB:

pub fn new_texture(&self, descriptor: &TextureDescriptorRef) -> Texture {
    unsafe { msg_send![self, newTextureWithDescriptor: descriptor] }
}

Seems like there's people already hitting those cases and wgpu now checks if the wrapped ptr is null, but that doesn't deal with the UB.

Let me know if I'm missing something, otherwise I can create a PR to perform the check and make create_texture return an Option<Texture>

[madsmtm]

Related: #282.

Note that in general, every new, alloc and init method can return NULL in Out Of Memory situations.

But that's a bit theoretical, I agree that the above is definitely a higher-priority issue.

[jesdazrez]

Yeah I think metal-rs should make & document a choice regarding Option<T> vs Result<T, _> vs panic! for those Out of Memory situations.

[cwfitzgerald]

wgpu would prefer Option so we can properly report OOM errors without panicing