On Sat, Mar 7, 2015 at 8:30 AM, William A. Rowe Jr. <[email protected]> wrote:
> On Thu, 5 Mar 2015 11:19:40 -0500
> Jim Jagielski <[email protected]> wrote:
>
>> After doing some additional research, I think that the
>> current implementation of NOT allowing duplicates (in
>> APR 1.5/1.6) is BROKEN. In trunk this is determined by
>> a flag w/ the insert statement, but the default *should*
>> be to allow dups.
>>
>> As such, I'd like to adjust 1.5/1.6 to the correct and,
>> afaict, canonical behavior.
>
> I don't believe we can do this, it defies all rational API compatibility
> revisioning policies of any project, anywhere.
There is a possibility to both preserve/restore the addn semantic to
skiplist_insert() and still have the add semantic in 1.5.x.
It would be to give apr_skiplist_insert_compare(), which exists in
1.5.x, the add semantic when its compare function arg is NULL.
Eg. for trunk:
Index: tables/apr_skiplist.c
===================================================================
--- tables/apr_skiplist.c (revision 1664776)
+++ tables/apr_skiplist.c (working copy)
@@ -517,29 +517,39 @@ static apr_skiplistnode *insert_compare(apr_skipli
APR_DECLARE(apr_skiplistnode *)
apr_skiplist_insert_compare(apr_skiplist *sl, void *data,
apr_skiplist_compare comp)
{
- return insert_compare(sl, data, comp, 1);
+ if (comp) {
+ return insert_compare(sl, data, comp, 0);
+ }
+ else if (sl->compare) {
+ return insert_compare(sl, data, sl->compare, 1);
+ }
+ else {
+ return NULL;
+ }
}
APR_DECLARE(apr_skiplistnode *) apr_skiplist_insert(apr_skiplist *sl,
void *data)
{
- if (!sl->compare) {
- return NULL;
- }
- return insert_compare(sl, data, sl->compare, 1);
+ return apr_skiplist_insert_compare(sl, data, sl->compare);
}
-APR_DECLARE(apr_skiplistnode *)
apr_skiplist_addne_compare(apr_skiplist *sl, void *data,
+APR_DECLARE(apr_skiplistnode *) apr_skiplist_add_compare(apr_skiplist
*sl, void *data,
apr_skiplist_compare comp)
{
- return insert_compare(sl, data, comp, 0);
+ if (comp) {
+ return insert_compare(sl, data, comp, 1);
+ }
+ else if (sl->compare) {
+ return insert_compare(sl, data, sl->compare, 1);
+ }
+ else {
+ return NULL;
+ }
}
-APR_DECLARE(apr_skiplistnode *) apr_skiplist_addne(apr_skiplist *sl,
void *data)
+APR_DECLARE(apr_skiplistnode *) apr_skiplist_add(apr_skiplist *sl, void *data)
{
- if (!sl->compare) {
- return NULL;
- }
- return insert_compare(sl, data, sl->compare, 0);
+ return apr_skiplist_add_compare(sl, data, sl->compare);
}
APR_DECLARE(int) apr_skiplist_remove(apr_skiplist *sl, void *data,
apr_skiplist_freefunc myfree)
--
And for 1.5.x:
Index: tables/apr_skiplist.c
===================================================================
--- tables/apr_skiplist.c (revision 1664805)
+++ tables/apr_skiplist.c (working copy)
@@ -394,8 +394,16 @@ APR_DECLARE(apr_skiplistnode *) apr_skiplist_inser
apr_skiplist_compare comp)
{
apr_skiplistnode *m, *p, *tmp, *ret = NULL;
- int nh = 1, ch;
+ int add = 0, nh = 1, ch;
+ if (!comp) {
+ if (!sl->compare) {
+ return NULL;
+ }
+ comp = sl->compare;
+ add = 1;
+ }
+
if (sl->preheight) {
while (nh < sl->preheight && get_b_rand()) {
nh++;
@@ -423,6 +431,11 @@ APR_DECLARE(apr_skiplistnode *) apr_skiplist_inser
*/
if (m->next) {
int compared = comp(data, m->next->data);
+ if (compared == 0 && !add) {
+ /* Keep the existing element(s) */
+ skiplist_qclear(&sl->stack_q);
+ return NULL;
+ }
if (compared > 0) {
m = m->next;
continue;
--
