<br><br><div class="gmail_quote">On Tue, Nov 25, 2008 at 9:22 AM, ron minnich <span dir="ltr"><<a href="mailto:rminnich@gmail.com">rminnich@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">On Tue, Nov 25, 2008 at 8:19 AM, Myles Watson <<a href="mailto:mylesgw@gmail.com">mylesgw@gmail.com</a>> wrote:<br>
>> >> +/* we should only ever delete simple nodes: nodes with no kids. */<br>
>> >> +void del_node(struct node *node)<br>
>> >> +{<br>
>> >> +     struct node *n;<br>
>> >> +     assert(! node->children);<br>
>> >> +     if (first_node == node) {<br>
>> >> +             first_node = node->next;<br>
>> >> +             free(node);<br>
>> >> +             return;<br>
>> >> +     }<br>
>> >> +     for_all_nodes(n) {<br>
>> >> +             if (n->next != node)<br>
>> >> +                     continue;<br>
>> >> +             n->next = node->next;<br>
>> >> +             n->next_sibling = node->next_sibling;<br>
>> ><br>
>> > Will this always be true?  It seems like you need to go through again to<br>
>> do<br>
>> > the sibling links right.<br>
>><br>
>> I don't think so because, at this point, n is prev(node to be deleted).<br>
>><br>
>> so I am setting prev(node)->next and next_sibling to node->next etc.<br>
>> Am I missing something?<br>
><br>
> I was thinking about the case where n wasn't the sibling of the node being<br>
> deleted.  For example, a first child being deleted.<br>
><br>
> It seems like you should at least check that n->next_sibling was node before<br>
> setting it to node->next_sibling.<br>
<br>
</div>I have code whiteout, can you write this correctly for me :-)</blockquote><div><br>I don't know if it's correct, but here's a first try:<br> </div><div>/* we should only ever delete simple nodes: nodes with no kids. */<br>
void del_node(struct node *node)<br>{<br>    struct node *n, *s;<br>    assert(! node->children);<br>    if (first_node == node) {<br>        first_node = node->next;<br>        free(node);<br>        return;<br>    }<br>
<br>    /* If the node is the first child, set parent->children link. */<br>    if (node->parent->children == node)<br>        node->parent->children = node->next_sibling;<br>    else {<br>        /* Remove from sibling list. */<br>
        for (n=node->parent->children; n->next_sibling;<br>             n=n->next_sibling)<br>            if (n->next_sibling == node)<br>                break;<br>        n->next_sibling = node->next_sibling;<br>
    }<br>        <br>    for_all_nodes(n) {<br>        if (n->next != node)<br>            continue;<br>        n->next = node->next;<br>        if (node == last_node)<br>            last_node = n;<br>        break;<br>
    }<br>    free(node);<br>}<br><br>It doesn't check to see if the child is really a child of its parent.  I figured that was a given.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">
>><br>
>> ><br>
>> > Do we guarantee that you will never be appending to a NULL list (first<br>
>> ==<br>
>> > NULL), I didn't see that check.<br>
>><br>
>> Here's my thinking on this. The guarantee is that there is always a<br>
>> root node -- we don't ever remove that.<br>
><br>
> I guess I'm confused here.  I was talking about when we call it from<br>
> fixup_properties with chipnodes as the list.  It looks like we could delete<br>
> all of them.  Do I have it backward?<br>
<br>
</div>yes. We should only ever delete mainboard nodes, not chip nodes.<font color="#888888"><br>
</font></blockquote></div><br>Right.  <br><br>Thanks,<br><br>Myles<br>