[Qemu-devel] Re: [PATCH 04/29] Introduce QDict

[Paolo Bonzini <bonzini@gnu.org>]

> +
> +/**
> + * tdb_hash(): based on the hash agorithm from gdbm, via tdb
> + * (from module-init-tools)
> + */
> +static unsigned int tdb_hash(const char *name)
> +{
> +    unsigned value;	/* Used to compute the hash value.  */
> +    unsigned   i;	/* Used to cycle through random values. */
> +
> +    /* Set the initial value from the key size. */
> +    for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++)
> +        value = (value + (((const unsigned char *)name)[i]<<  (i*5 % 24)));

This is a _bad_ hash function, especially because 5 is exactly the bit 
that is flipped between lowercase and uppercase.  So "aa" would collide 
with "Ab".

Besides, since the function is additive, you can initialize the value to 
0 and add "0x238F13AF * i" at the end (but this is just showing off...).

Here is a very simple but good hash function:

   uintptr_t hashVal = 1497032417;    /* arbitrary value */

   while (len--)
     {
       hashVal += *str++;
       hashVal += (hashVal << 10);
       hashVal ^= (hashVal >> 6);
     }

   return 1103515243 * value;

> +/**
> + * qdict_add(): Add a new object into the dictionary
> + *
> + * Add the pair 'key:value' into qdict. Does nothing if 'key' already
> + * exist.
> + *
> + * NOTE: this function 'steals' a reference to 'value'

Using my proposed nomenclature, "ownership of value is transferred to 
the QDict".

> +void qdict_add_qint(QDict *qdict, const char *key, QInt *qi)

I agree with Avi that these do not add much.  If anything, make wrappers 
that accept C datatypes.

> +/**
> + * qdict_get(): Lookup for a given 'key'
> + *
> + * Return borrowed reference to QObject if 'key' exists,
> + * NULL otherwise.

Return a weak reference to the QObject associated with 'key' if
the key is present in the dictionary, NULL otherwise.

> +/**
> + * qdict_del(): Delete a 'key:value' pair from the dictionary
> + *
> + * This will destroy all data allocated by this entry.

... decrementing the reference count of the QObject associated to 'key'.

Paolo