On Fri, Jul 11, 2014 at 9:58 PM, Yann Ylavic <[email protected]> wrote:
> On Fri, Jul 11, 2014 at 9:57 PM, Yann Ylavic <[email protected]> wrote:
>> On Fri, Jul 11, 2014 at 9:38 PM, Jim Jagielski <[email protected]> wrote:
>>>
>>> On Jul 11, 2014, at 12:29 PM, Yann Ylavic <[email protected]> wrote:
>>>
>>>>>
>>>>> Maybe we could also leave skiplist as is and introduce a new type
>>>>> (skipmap?) that would be ordered and that would take both key and
>>>>> value as arguments (in the relevant functions)...
>>>>
>>>
>>> Yeah... good idea.
>>
>> OK, will propose that.
>>
>> I'll commit skiplist's size computation bugfix included in the current
>> patch, and maybe the stack reuse (to avoid malloc()s for each
>> insert()).
>
> And the tests.
There also seem to be some doubtful code in apr_skiplist_remove_all(),
addressed by the following patch :
Index: tables/apr_skiplist.c
===================================================================
--- tables/apr_skiplist.c (revision 1609655)
+++ tables/apr_skiplist.c (working copy)
@@ -591,16 +591,17 @@ APR_DECLARE(void) apr_skiplist_remove_all(apr_skip
m = sl->bottom;
while (m) {
p = m->next;
- if (p && myfree && p->data)
- myfree(p->data);
+ if (myfree && m->data)
+ myfree(m->data);
while (m) {
u = m->up;
- apr_skiplist_free(sl, p);
+ apr_skiplist_free(sl, m);
m = u;
}
m = p;
}
sl->top = sl->bottom = NULL;
+ sl->topend = sl->bottomend = NULL;
sl->height = 0;
sl->size = 0;
}
Do you agree?
