Re: commit: ldap/servers/slapd/back-ldbm attr.c

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

Pierangelo Masarati writes:
> I'm not sure we can rely on (x) > (y) returning 1 if true.

We can.  ANSI 3.3.8 'Relational operators' ends with:

  Each of the operators < (less than), > (greater than), (...) shall
  yield 1 if the specified relation is true and 0 if it is false.  The
  result has type int.

> int
> main(void)
> {
>         void *x = 0x0;

This is just another way to say void *x = NULL, and you can't subtract
null pointers portably.  Better use x = (void*)1, at least.
However,

>         unsigned long long i;

What if long long is not supported?

>         for (i = 1; i <= ULONG_MAX; (i <<= 1) ) {
>                 void            *y = (void *)i;

This only tests pointers that fit in an unsigned long.  If the machine
supports pointers wider than long, malloc can return pointers you didn't
test.  Loop until i == 0 instead, that'll cover pointers as wide as long
long.  I guess they are unlikely to be wider than that.

>                 assert( y > x ) ;
>                 assert( dplus > 0 );
>                 assert( dminus < 0 );

You assume a memory model of continuous linear memory and that casting
integers to pointers reflects that.  On a host where that is not true,
these tests could succeed by accident for those particular pointer
values.  I admit it seems unlikely for all the tests to succeed, though.
And we are already halfway assuming such a memory model, so maybe it's
all right.  But it does leave me uncomfortable.  I don't think you
should do this unless profiling shows that we gain something from it.

> on my system this fails when trying to use i = 2147483648
> which is 1 more than MAX_INT.  If we can restrict the
> addressable space, e.g. by knowning all the memory comes
> from one malloc, then it's fine

Does it?  I thought it could come from different calls to malloc.

> (what about if comparing with NULL ?).

Don't, except comparing for equality.

>> int PTRCMP(const void *x, const void *y)
>> {
>>     return x < y ? -1 : x > y;
>> }
>
> with a test
>
> assert( ( x > y ) > 0 );

You mean assert( ( x > y ) >= 0 )?

-- 
Hallvard