[Date Prev][Date Next]
Re: commit: ldap/servers/slapd/back-ldbm attr.c
> hyc@OpenLDAP.org writes:
>> Modified Files:
>> attr.c 1.33 -> 1.34
> Also back-ldap/bind.c 1.52 -> 1.53, back-bdb/attr.c 1.13 -> 1.14
>> Log Message:
>> Fix AVL comparisons
>> - return desc - a->ai_desc;
>> + return (unsigned)desc - (unsigned)a->ai_desc;
> I don't know what this is supposed to fix, but:
> (a) Both new and original code gives the wrong result or signals an
> overflow for pointers to memory locations that are more than
> INT_MAX bytes or so apart.
> (b) Pointer comparison is only defined between pointers to the same
> array. The result of the original code is undefined if desc and
> a->ai_desc point to the results of different malloc operations.
> (c) The result of casting a pointer to an integer is implementation-
> defined. I think it might e.g. return 42 for any pointer.
> There is no way to fix all three problems.
> This fixes (a) and (c):
> if (desc < a->ai_desc)
> return -1;
> return (desc != a-ai_desc);
> This has a better chance than the committed code of fixing (a) and (b),
> but can still fail if pointers are wider than unsigned long:
> if ((unsigned long)desc < (unsigned long)a->ai_desc)
> return -1;
> return ((unsigned long)desc != (unsigned long)a-ai_desc);
> Take your pick. I vote for fixing (a) and (c) since that at least fixes
> these problems completely, which the fix for (b) is not guaranteed. If
> we meet an implementation where (b) is a problem, we can fix (b), or
> rewrite these functions completely so they don't compare pointers at
> all, with or without casts to integers.
Howard and I discussed the problem, recently; we came out with
the conplusion that we definitely need a portable way to do
this comparison, which is basically
#define PTRCMP(x,y) ((x) - (y))
#define PTRCMP(x,y) ((x) > (y) ? 1 : ((x) < (y) ? -1 : 0)
where SAFE is: ptrdiff_t is:
- large enough to contain +/- the space addressable
by our pointers
- its cast to int (since AVL cmp funcs return int)
doesn't lose the sign bit
If there are no objections I might start
implementing this soon.
Of course, we may simply stay with !SAFE;
we'd basically waste one comparison ...