Should umfPutIPCHandle and umfOpenIPCHandle take const handle data pointers?

[steffenlarsen]

Rationale

umfPutIPCHandle and umfOpenIPCHandle currently take an umf_ipc_handle_t, but since the handles are replicable across processes I would not expect these functions to change the data contained in the handles. Could these be changed to take const handles?

API Changes

Change from

umf_result_t umfPutIPCHandle(umf_ipc_handle_t ipcHandle);
umf_result_t umfOpenIPCHandle(umf_ipc_handler_handle_t hIPCHandler, umf_ipc_handle_t ipcHandle, void **ptr)

to

typedef const struct umf_ipc_data_t *umf_const_ipc_handle_t;

umf_result_t umfPutIPCHandle(umf_const_ipc_handle_t ipcHandle);
umf_result_t umfOpenIPCHandle(umf_ipc_handler_handle_t hIPCHandler, umf_const_ipc_handle_t ipcHandle, void **ptr)

[lplewa]

You’re right — but since we’re already past 1.0, this would be an API break, so we can’t change it now. If we ever do a 2.0 release, we can add the const, but it’s not worth a major version bump on its own.

Note: if someone implements this in the future, keep in mind that
const umf_ipc_handle_t makes the pointer itself const (like int * const), not the data it points to (const int *).
To make the data const, a new type umf_const_ipc_handle_t would need to be added.

[steffenlarsen]

You’re right — but since we’re already past 1.0, this would be an API break, so we can’t change it now. If we ever do a 2.0 release, we can add the const, but it’s not worth a major version bump on its own.

An option could be to define the const variants in addition to the non-const versions, then redirect the non-const to call the const functions. Since having const arguments is a stronger guarantee, both should then work, unlike if it had been the other way around. If needed, the old versions could be deprecated and then removed in a future 2.0 version (if ever done.)

Note: if someone implements this in the future, keep in mind that
const umf_ipc_handle_t makes the pointer itself const (like int * const), not the data it points to (const int *).
To make the data const, a new type umf_const_ipc_handle_t would need to be added.

Good point! I have amended the suggestion.

[lplewa]

I’ve been thinking about this a bit more — before the 1.0 release, I actually added a bunch of consts to the UMF API, but I skipped this one. Now I remember why.

We can’t make the handle in umfPutIPCHandle const, because the data becomes invalid after that call — we’re effectively modifying it. In C, all “free”-like functions take non-const pointers, since they modify the memory state (turning the pointed memory into an undefined state).
Also since umfPutIPCHandle internally calls free, we’d have to do a dirty cast to “unconst” the handle before freeing it.

For umfOpenIPCHandle, though, it could technically take a const pointer.
But both options to support that are… well, ugly, and I’m not sure this missing const is serious enough to justify the complications in the header:

umf_result_t umfOpenIPCHandle(umf_ipc_handler_handle_t hIPCHandler,
                              umf_ipc_handle_t ipcHandle, void **ptr);
umf_result_t umfConstOpenIPCHandle(umf_ipc_handler_handle_t hIPCHandler,
                                   umf_const_ipc_handle_t ipcHandle, void **ptr);

or

#ifdef UMF_CONST_OPEN_IPC_HANDLE
umf_result_t umfOpenIPCHandle(umf_ipc_handler_handle_t hIPCHandler,
                              umf_const_ipc_handle_t ipcHandle, void **ptr);
#else
umf_result_t umfOpenIPCHandle(umf_ipc_handler_handle_t hIPCHandler,
                              umf_ipc_handle_t ipcHandle, void **ptr);
#endif

Anyway, missing consts are a very common thing in C, so I’d say this one isn’t worth complicating the header for.
Is this const for some reason very important for you?