[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: nonstandard assumptions in the code



Kurt D. Zeilenga writes:
>At 11:01 AM 5/30/2003, Hallvard B Furuseth wrote:
>>
>> Here are the nonstandard assumptions I have found.
>> (...)


>> * no padding bits in 'unsigned int'
>>   (required by back-bdb/dbcache.c:bdb_db_hash() - don't know why)
>> (...)
> These should be fixed.

Howard, this is your code, from when you fixed ITS#1726.  Something
about endianness, perhaps?  If not, we can use this function body,
roughly from my original patch.  I have dropped the ugly if-test for
whether or not memcpy can be used, since I doubt the code is that time-
critical.

{
	u_int32_t i, ret = 0;

	if ( length > sizeof(u_int32_t) )
		length = sizeof(u_int32_t);

	for( i = 0; i < length; i++ )
		ret = (ret << 8) + ((const unsigned char *)bytes)[i];
	return ret;
}

>> * can cast function pointers to 'void *' and back
>>   (at least the threading stuff)
>
> We should be able to clean most of that up.

Unfortunately, this assumption is also in the code

   ber_set_option(, LBER_OPT_LOG_<PROC/PRINT_FN>, (void *) function )

The correct fix would be to rename the options and pass a pointer to a
variable containing a pointer to the function:

    BER_LOG_PRINT_FN func_ptr = some_function;
    ber_set_option(, LBER_OPT_LOG_PRINT_FN, (void*) &func_ptr );

but that's pretty cumbersome, so I don't know if we want to do that?

(BTW, there is also an unused LBER_OPT_ERROR_FN option.)


Another non-ANSI thing: LBER_OPT_ON and LDAP_OPT_ON are #defined as
((void*)1).  There is no guarantee that this is != NULL (the *_OPT_OFF
options).  I'll install this fix unless someone protests:

  liblber/options.c:
    char lber_pvt_opt_on; /* used to get a non-NULL address for *_OPT_ON */

  include/lber.h:
    extern char lber_pvt_opt_on;
    #define LBER_OPT_ON ((void *) &lber_pvt_opt_on)

  include/ldap.h:
    #define LDAP_OPT_ON ((void *) &lber_pvt_opt_on)

-- 
Hallvard