Question

related post: c circular double linked-list delete_node - iterate traverses deleted node on first pass after delete

All, implementing a search for node contianing line number 'x' prior to deleting that node, I ran across a problem where both forward and reverse searches identify the proper node, but the pointer for the caller's node address is reported differently by the reverse search than for the forward? This applies to the last node (hightest line number) only. If only the forwrd search is used (pba_fwd_iter_test), then the last node is deleted properly. However, if the reverse search is used (pba_rev_iter_test), then the address set by "(victim->next)->prev = victim->prev;" is incorrect, it sets "(victim->next)->prev = (victim->next)->prev". For example, arriving at the end node with a reverse search and then preforming the delete_node results in the following:

49: 7 - (line to delete) This is a line of text that is somewhere around 50 to 80 characters in length

48 - prev: 0x604a80  cur: 0x604b10  next: 0x604ba0
49 - prev: 0x604b10  cur: 0x604ba0  next: 0x603010  <-- delete_node
 0 - prev: 0x604ba0  cur: 0x603010  next: 0x6030a0

48 - prev: 0x604a80  cur: 0x604b10  next: 0x603010
49 - prev: 0x604b10  cur: 0x604ba0  next: 0x603010  <-- (node deleted)
 0 - prev: 0x603010  cur: 0x603010  next: 0x6030a0
              \_______________\______ Error (should be prev: 0x604b10)

@WhosCraig graciously helped with the delete_node function which works fine, but I cannot figure out why when locating the same node with the reverse search results in the delete_node failing to set "(victim->next)->prev = victim->prev;" properly. As a test of the reverse search, I simply stepped one additional node toward the beginning and then went forward one node back to the node in question and then the delete_node worked fine. (simply an additional: list = &(*list)->prev; list = &(*list)->next;. So the issue has something to do with the pointer state when arriving at the end-node with a reverse search rather than a forward seach -- that is what I need help figuring out. Here is the output of the pointer addresses following both forward and reverse searchs, as well as following the quick ->prev ->next:

=========== pba_fwd_iter_test() ===========
passing list = &(*list)->next to tstpptr (0x605b28)
tstpptr(): list     : 0x605b28
tstpptr(): &list    : 0x7ffff14633a8
tstpptr(): *list    : 0x605ba0
tstpptr(): &(*list) : 0x605b28  <- caller's address reported
tstpptr(): &(**list): 0x605ba0     with forward search

tstpptr(): &(*list)->next : 0x605bb8

=========== pba_rev_iter_test() ===========
passing list = &(*list)->next to tstpptr (0x604020)
tstpptr(): list     : 0x604020
tstpptr(): &list    : 0x7ffff14633a8
tstpptr(): *list    : 0x605ba0
tstpptr(): &(*list) : 0x604020  <- caller's address reported
tstpptr(): &(**list): 0x605ba0     with reverse search

tstpptr(): &(*list)->next : 0x605bb8

passing list = &(*list)->next to tstpptr (0x605b28)
tstpptr(): list     : 0x605b28
tstpptr(): &list    : 0x7ffff14633a8
tstpptr(): *list    : 0x605ba0
tstpptr(): &(*list) : 0x605b28  <- caller's address reported after
tstpptr(): &(**list): 0x605ba0     &(*list)->prev; &(*list)->next

tstpptr(): &(*list)->next : 0x605bb8

The following are the relevant code snippets with the link to the full source at the beginning. Thank you for any help you can provide:

/*
full source: http://www.3111skyline.com/dl/dev/prg/src/ll-double-cir-1.c.txt
*/

struct record
{
char *line;
int lineno;
int linetype;
struct record *prev;
struct record *next;
};
typedef struct record rec;

void  // traverse in fwd direction to find hightest line no.
pba_fwd_iter_test (rec **list, int num);
void  // traverse in rev direction to find hightest line no.
pba_rev_iter_test (rec **list, int num);
void  // dump the pointers for examination
tstpptr (rec **list);

int main (int argc, char *argv[]) {
// <snip> fill struct with 50 records for testing (lineno '0' based 0-49)
pba_fwd_iter_test (&textfile, 49);
pba_rev_iter_test (&textfile, 49);
return 0;
}

void
pba_fwd_iter_test (rec **list, int num) {
printf ("=========== %s() ===========\n",__func__);
int linemax = getmaxline (*list);
int iterno = 0;
while (((*list)->lineno != num) && (iterno <= linemax)) {
    iterno++;
    list = &(*list)->next;
}
printf ("passing list = &(*list)->next to tstpptr (%p)\n", list);
tstpptr (list);
}

void
pba_rev_iter_test (rec **list, int num) {
printf ("=========== %s() ===========\n",__func__);
int linemax = getmaxline (*list);
int iterno = 0;
while (((*list)->lineno != num) && (iterno <= linemax)) {
    iterno++;
    list = &(*list)->prev;
}
printf ("passing list = &(*list)->next to tstpptr (%p)\n", list);
tstpptr (list);
// increment prev then next and check ptr values again
list = &(*list)->prev;
list = &(*list)->next;
printf ("passing list = &(*list)->next to tstpptr (%p)\n", list);
tstpptr (list);
}

void
tstpptr (rec **list) {
fprintf (stdout, "%s(): list      : %p\n", __func__, list);
fprintf (stdout, "%s(): &list     : %p\n", __func__, &list);
fprintf (stdout, "%s(): *list     : %p\n", __func__, *list);
fprintf (stdout, "%s(): &(*list)  : %p\n", __func__, &(*list));
fprintf (stdout, "%s(): &(**list) : %p\n\n", __func__, &(**list));
fprintf (stdout, "%s(): &(*list)->next : %p\n\n", __func__, &(*list)->next);
}
Was it helpful?

Solution

I think I see the problem - I don't think there is one. The important value is *list, which is the same in all cases. I think printing out list and &list etc is just clouding the issue.

In your forward iterator, list is pointing to the location of item #48's next variable.

In your backward iterator, list is pointing to the location of item #0's prev variable.

In both cases *list is pointing to the correct item #49.

Both functions would be much simpler if they just took a rec *, rather than a rec **, then it would be more obvious that taking the address of the list variable isn't what you want.

OTHER TIPS

I took your code from the 3111skyline.com site, but it does not call the delete functions at all (or a lot of the other functions). I added a function validate_list() which confirms that your list is constructed correctly. And I created a loop to delete rows from your list, like this:

static void validate_list(rec *list)
{
    assert(list != 0);
    assert(list->next != 0);
    assert(list->prev != 0);
    rec *iter = list;
    do
    {
        assert(iter != 0);
        assert(iter->next != 0);
        assert(iter->prev != 0);
        assert(iter == iter->next->prev);
        assert(iter == iter->prev->next);
        printf("Node: %p (n = %p; p = %p) %2d [%s]\n",
               (void *)iter, (void *)iter->next,
               (void *)iter->prev, iter->lineno, iter->line);
        iter = iter->next;
    } while (iter != list);
}

int main(void)
{
    rec *textfile = fillrecord();
    validate_list(textfile);

    for (int i = 0; i < 49; i++)
    {
        delete_node(&textfile, (i * 13 + 31) % (50 - i));
        validate_list(textfile);
    }
    return 0;
}

It all seems to work OK for me. The next step would be to replace the deterministic (but not particularly obvious) sequence generated by the expression with random numbers to check that it works that way too, but the code seems clean to me.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top