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

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

masarati@aero.polimi.it writes:
> 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?

Yes, I explained how.  Test (0xdead0000 == (unsigned long long)rc)
instead, then you'll see the problem with casting to a wider type than int.

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

Let's see, this makes explicit the integer promotion which already
happened since slap_mask_t is unsigned long and *iptr is int:

-	if ( *iptr == aux[i].mask ) {
+	if ( (slap_mask_t)*iptr == aux[i].mask ) {

Can (*iptr & 0x80000000) == 0x80000000?  If so, this code was buggy just
the same way your 0xdead0000 patch is - and the new warning just hides
the bug.  Casting both *iptr and mask to unsigned int would be better,
I expect.

The same applies to the next patch, for ival.


The real fix would be to use unsigned types, and preferably the same
types everywhere, but changing the signedness of flags requres careful
review of all code using those flags, looking for effects like this
patch.

-- 
Hallvard