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

Re: commit: ldap/servers/slapd config.c

ando@OpenLDAP.org writes:
> 	config.c  1.506 -> 1.507
> silence signedness warnings
> -			} else if ( rc == ARG_BAD_CONF ) {
> +			} else if ( (unsigned long)rc == ARG_BAD_CONF ) {

That breaks working code.  Never shut up signedness warnings without
working through what the expressions do, it's too easy to go wrong.

rc is an int.
ARG_BAD_CONF = 0xdead0000: an unsigned int if int is 32-bit.
Consider 32-bit int, 64-bit long, with rc == (int)ARG_BAD_CONF:
rc < 0 so (unsigned long)rc sign-extends to 0xffffffffdead0000,
but ARG_BAD_CONF itself is positive so it is not sign-extended.

The old code worked as far as I can tell: It converted rc to unsigned
int, yielding the original ARG_BAD_CONF.

The same functinality change happens with the other changes below that
one, I haven't looked at whether that is a fix of broken code or if it
breaks working code.  (Or it is breaks broken code in a new way:-)