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

[masarati@aero.polimi.it]

> 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.

My PC is 32-bit, so int is 32-bit.  If I do

        int rc = 0xdead0000;

(0xdead0000 == (unsigned long)rc) is true.  Am I doing anything wrong?

> 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:-)

The other changes should eb just fine, since the values of the signed
variables are expected to be non-negative, and this test is done without
casting.

p.