Re: [PATCH] generic lookup dictionary (hash) - extends gwlib/dict.[ch]

Hi,

I don't know now how to use this code but code reviews show:

1) gw_dict_ops_t doesn't define function to destroy key. Because key is  
not Octstr anymore you need to know which function to call.

2) because of (1) you don't destroy keys => memleak

3)
+List *gw_dict_keys(gw_dict_t *dict)
+{
+    List *list;
+    Item *item;
+    long i, j;
+
+    list = gwlist_create();
+
+    lock(dict);
+    for (i = 0; i < dict->size; ++i) {
+        if (dict->tab[i] == NULL)
+            continue;
+        for (j = 0; j < gwlist_len(dict->tab[i]); ++j) {
+            item = gwlist_get(dict->tab[i], j);
+            gwlist_append(list, octstr_duplicate(item->key));

why octstr_duplicate? key is not Octstr anymore... copy&paste ;)
for gw_dict_keys we need duplicate function for key probably in  
gw_dict_ops_t...

+        }
+    }
+    unlock(dict);
+
+    return list;
+}

Am 31.01.2007, 22:18 Uhr, schrieb Stipe Tolj <[email protected]>:

> Hi list,
>
> attached is an enhanced version of our gwlib/dict.[ch], now called
> gwlib/gw-dict.[ch] that can used generic key types.
>
> Generic means void* key actually, and now Octstr* key as current Dict  
> does. Why?
> Because under certain conditions it's too expensive to do all the string
> mangling stuff via octstr_foobar() routines just to get a unsigned long  
> that is
> used internally for the hash table position.
>
> To achive the abstraction of the key type, the gw_dict_t user needs to  
> have a
> struct of type gw_dict_ops_t that defines three functions:
>
>    .destroy - function applied to all void* item in the hash when
>               the dict is beeing destroyed
>    .key_to_index - function mapping a given void* key to a unsigned long
>                    that is used to calculate the hash table position
>    .item_has_key - function used for gwlist_search() as compare function
>                    to give a result if the given key matches the stored  
> item
>
> See also attched test/test_gw_dict.c which is a copy of test/test_dict.c  
> but
> uses of course the new gw_dict_t* instead of Dict*.
>
> I haven't yet reviewed where we can use this new approach of a  
> dictionary in the
> gw/ code, but I'm at least sure we'll have some usage for it. I use it  
> for a
> enhanced way in dealing with WSP session machines for wapbox et al.
>
> Please review and vote.
>
> Stipe
>
> -------------------------------------------------------------------
> Kölner Landstrasse 419
> 40589 DÃŒsseldorf, NRW, Germany
>
> tolj.org system architecture      Kannel Software Foundation (KSF)
> http://www.tolj.org/              http://www.kannel.org/
>
> mailto:st_{at}_tolj.org           mailto:stolj_{at}_kannel.org
> -------------------------------------------------------------------
>



-- 
Thanks,
Alex