#155 slapi void** function arguments not generally portable
Closed: invalid 2 years ago Opened 7 years ago by mkosek.


Description of problem:

slapi_ch_free()'s prototype makes it unportable to hosts where
some pointer types have different representation than void*:
    slapi_ch_free((void **)&ptr);
will access ptr as a void*, regardless of ptr's real type.
Pity to do that for something as trivial as nulling the pointer
after freeing it.

(Actually I encountered this in OpenLDAP which also implements
slapi, but there isn't much point in "fixing" your API there
when that makes it incompatible with yours.)

Chris Torek gives an example of how this can break even for a
mostly normal architecture:
The Data General Eclipse had word pointers as the native pointer
format, so casting int* to char* (this was before void*:-) had to
left-shift the address.  If int*foo; contained address 666, that
was word#666.  free(*(void**)foo) would free byte#666 = word#333.

The void** type of the function for slapi_be_getentrypoint() is
similarly broken, but if it knows the type of the function pointer
which the argument points to, then it can convert the argument
back to a pointer to that function pointer type before using it.

The void*[] array of function pointers for slapi_apib_...() is
worse, since a void* may not be able to hold a function pointer
according to the C standard.  It would have been OK to use
    typedef (*slapi_fn)(void);
and make the api an array of function pointers cast to slapi_fn.

This is probably not a problem now since dlsym() works, but
Open Group apparently plans to provide a replacement function
for this reason - see RATIONALE in

batch update moving tickets to future

set default ticket origin to Community

Added initial screened field value.

Metadata Update from @nkinder:
- Issue set to the milestone: FUTURE

2 years ago

This is tricky to fix as we have this in our public api, and we rely on some of these quirks. we would need to rethink our whole ch_malloc implementation in light of this Ithink .

I don't know if it's possible to really change this, and making it change breaks one our behaviours which is that a pointer that is freed, is NULLed.

At the least, this doesn't break valgrind or other checks.

Finally, this was closed in BZ so closing here too.

Metadata Update from @firstyear:
- Custom field component reset (from Server - Plugins)
- Custom field reviewstatus adjusted to new (was: Needs Review)
- Issue close_status updated to: invalid
- Issue status updated to: Closed (was: Open)

2 years ago

Login to comment on this ticket.