[PULL] [xorg/xserver] XRes v1.2

[Erkki Seppala]

Taking this back to list, in case it collects some more comments.

Context: I sent a PULL request of 
git://gitorious.org/erkkise/fdo-xserver.git client-tracking-v2 to Keith, 
given that the previous update had not risen any discussion.

On 04.04.2011 21:51, Keith Packard wrote:
> On Mon, 04 Apr 2011 12:14:19 +0300, Erkki Seppala<erkki.seppala at vincit.fi>  wrote:
>
>>         dix: add hashing functions to resource.h for others to use.
>
> CompareResourceID should not be in resource.c
> You should not create a new 'HashResourceID' function with a weird
> signature. Instead, you should change the function signature of the
> existing Hash function and publish that. The new signature should be:
>
> int Hash(XID id, int numBits);

Changed the interface - I still call the function HashResourceID, 
though, because it is really usable only for hashing resource IDs. 
Internally resource.c now uses the same static function Hash implemented 
in terms of HashResourceID.

The signature of my hashing function was intended to be uniform so that 
it would work directly with the generic hash table implementation, but 
given that it has now moved from dix to Xext, it doesn't seem that 
important anymore. So the uniform interface was moved to 
Xext/hashtable.c as ht_resourceid_hash/compare.

> Your current implementation has HashResourceID returning 'unsigned' with
> '0' indicating an error while Hash returns 'int' with -1 indicating an
> error. Are you positive that 0 is never a legal hash value? -1 is
> clearly OK as numBits is smaller than 31.

The unsigned return value was because it doesn't make sense to me for a 
hashing operation to fail. Indeed, the return value is never checked in 
resource.c and should the function ever return -1, it will likely cause 
random data corruption or crashes. Perhaps it should be an assert instead.

Returning 0 from ht_resourceid_compare just makes the distribution 
uneven at worst, so it doesn't matter if the hash value 0 is legal or 
not. But I fixed this anyway, because uneven distribution can become a 
hidden problem, so now it uses a generic hash function in that situation.

Other updates I did:
Xext/hashtable.c one_at_a_time accepts a const void pointer instead of 
of const char pointer
Xext/hashtable.c ht_dump_contents for dumping the contents of the hash 
table for debugging purposes

Thanks for the comments! Gitorious is updated.