In the C API, conduit_node is void which allows for surprising conversions
#782
Comments
|
I'd like to investigate fixing this in some way. Who else should be involved in such a discussion? |
|
@mathstuf sorry for the delay. I agree this is an improvement. First step would be making the change and seeing if there are any surprises in CI. |
mathstuf
added a commit
to mathstuf/conduit
that referenced
this issue
Jul 19, 2021
Using `typedef void` meant that the APIs were all taking `void*` which in turn means that any pointer is valid to pass to these types. Instead, use a unique opaque type so that callers must either do the unsafe casting themselves or use the `cpp_to_c` APIs to do the casting the intended way. The actual definition of the structs themselves are empty; they are just used it as a way to smuggle the C++ pointer around under an opaque name. Fixes: LLNL#782
|
See #795. The fix is pretty simple and doesn't seem to break the build here at least. Hopefully Catalyst and ParaView have been as careful… |
mathstuf
added a commit
to mathstuf/conduit
that referenced
this issue
Jul 20, 2021
Using `typedef void` meant that the APIs were all taking `void*` which in turn means that any pointer is valid to pass to these types. Instead, use a unique opaque type so that callers must either do the unsafe casting themselves or use the `cpp_to_c` APIs to do the casting the intended way. The actual definition of the structs themselves are empty; they are just used it as a way to smuggle the C++ pointer around under an opaque name. Fixes: LLNL#782
cyrush
pushed a commit
that referenced
this issue
Jul 20, 2021
Using `typedef void` meant that the APIs were all taking `void*` which in turn means that any pointer is valid to pass to these types. Instead, use a unique opaque type so that callers must either do the unsafe casting themselves or use the `cpp_to_c` APIs to do the casting the intended way. The actual definition of the structs themselves are empty; they are just used it as a way to smuggle the C++ pointer around under an opaque name. Fixes: #782
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, the C API does:
which means the APIs are all really:
to the compiler. This means that:
typechecks and compiles just fine (and probably crashes or causes some other kinds of fireworks at runtime). Instead, it should be something like:
so that there is a distinct type associated with the call and the compiler can at least complain that types don't match (or that the user has to explicitly do a C cast or
reinterpret_castin which case the fault is squarely in their code).Cc: @cyrush @utkarshayachit
The text was updated successfully, but these errors were encountered: