This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: does gc_list_merge merge properly?
Type: behavior Stage:
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: mark.dickinson, martenjan
Priority: normal Keywords:

Created on 2013-11-05 11:14 by martenjan, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (3)
msg202206 - (view) Author: (martenjan) Date: 2013-11-05 11:14
The code for gc_list_merge is given in gcmodule.c. I retrieved it from
http://hg.python.org/cpython/file/tip/Modules/gcmodule.c#l287

The code seems to merge list `from` incompletely: the first entry of `from` is omitted in the merged list. 

The issue is in line 295:   tail->gc.gc_next = from->gc.gc_next;
I fixed it as :             tail->gc.gc_next = from;

Please check if my analysis is correct.


See below for the context.

Original: lines 287 to 301

/* append list `from` onto list `to`; `from` becomes an empty list */
static void
gc_list_merge(PyGC_Head *from, PyGC_Head *to)
{
    PyGC_Head *tail;
    assert(from != to);
    if (!gc_list_is_empty(from)) {
        tail = to->gc.gc_prev;
        tail->gc.gc_next = from->gc.gc_next;
        tail->gc.gc_next->gc.gc_prev = tail;
        to->gc.gc_prev = from->gc.gc_prev;
        to->gc.gc_prev->gc.gc_next = to;
    }
    gc_list_init(from);
}


Fix:
/* append list `from` onto list `to`; `from` becomes an empty list */
static void
gc_list_merge(PyGC_Head *from, PyGC_Head *to)
{
    PyGC_Head *tail;
    assert(from != to);
    if (!gc_list_is_empty(from)) {
        tail = to->gc.gc_prev;
        tail->gc.gc_next = from;
        tail->gc.gc_next->gc.gc_prev = tail;
        to->gc.gc_prev = from->gc.gc_prev;
        to->gc.gc_prev->gc.gc_next = to;
    }
    gc_list_init(from);
}
msg202209 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-11-05 13:26
The current code is correct.  Note that `from` is the list header, and is not attached to a gc-tracked object.  We want to splice the last non-header element of `to` (`to->gc.gc_prev`) to the first non-header element of `from` (`from->gc.gc_next`).

Did you try running the test suite with your change?  It would be quite a feat for a bug this fundamental to have made it this far in 2.7 without anyone noticing. :-)

Closing as invalid.
msg202218 - (view) Author: (martenjan) Date: 2013-11-05 15:29
Thanks for your explanation
History
Date User Action Args
2022-04-11 14:57:53adminsetgithub: 63702
2013-11-05 15:29:22martenjansetmessages: + msg202218
2013-11-05 13:26:55mark.dickinsonsetstatus: open -> closed

nosy: + mark.dickinson
messages: + msg202209

resolution: not a bug
2013-11-05 11:14:05martenjancreate