go-gl / gl

Go bindings for OpenGL (generated via glow)
MIT License
1.07k stars 74 forks source link

Ptr is subtly wrong if passed a pointer to a slice #32

Closed UserAB1236872 closed 8 years ago

UserAB1236872 commented 8 years ago

Currently, if reflect.Ptr is hit, it will check if the pointer is to a "known primitive" of some sort, and if so, grab the address.

The problem is, this will silently fail if anything else is passed in. No panic, it will just return the default value for an unsafe Pointer, which is null, which will probably segfault, do nothing, or do something really weird.

We should either not check if the value is primitive, panic on a pointer to a non-primitive, or else handle that case in more fine-grained detail.

This may or may not be affected by changes made for #31.

dmitshur commented 8 years ago

I think this is still an issue, and it might've caused (in part) #37.

Also see https://github.com/go-gl/gl/issues/37#issuecomment-190819444.

UserAB1236872 commented 8 years ago

My preference is to panic. I could see the argument for adding special casing to, specifically, *[]T to automatically dereference and then do what we already do for Ptr([]T), but whether we do that or not, panicking is probably appropriate here for other types. (It would be way too breaking and ugly to introduce an error here).

dmitshur commented 8 years ago

I agree with panic (with a helpful message of why). It's invalid API usage that will lead to program crashes anyway. Panicking with the right message will help spot and resolve the issue immediately.

dmitshur commented 8 years ago

This is fixed in https://github.com/go-gl/glow/commit/930bfae4963568b3463b01f39e5d07463d0214eb, just need to regenerate this repo now.