[coreboot] dt compiler patch

ron minnich rminnich at gmail.com
Tue Nov 25 17:22:10 CET 2008


On Tue, Nov 25, 2008 at 8:19 AM, Myles Watson <mylesgw at gmail.com> wrote:
>> >> +/* we should only ever delete simple nodes: nodes with no kids. */
>> >> +void del_node(struct node *node)
>> >> +{
>> >> +     struct node *n;
>> >> +     assert(! node->children);
>> >> +     if (first_node == node) {
>> >> +             first_node = node->next;
>> >> +             free(node);
>> >> +             return;
>> >> +     }
>> >> +     for_all_nodes(n) {
>> >> +             if (n->next != node)
>> >> +                     continue;
>> >> +             n->next = node->next;
>> >> +             n->next_sibling = node->next_sibling;
>> >
>> > Will this always be true?  It seems like you need to go through again to
>> do
>> > the sibling links right.
>>
>> I don't think so because, at this point, n is prev(node to be deleted).
>>
>> so I am setting prev(node)->next and next_sibling to node->next etc.
>> Am I missing something?
>
> I was thinking about the case where n wasn't the sibling of the node being
> deleted.  For example, a first child being deleted.
>
> It seems like you should at least check that n->next_sibling was node before
> setting it to node->next_sibling.

I have code whiteout, can you write this correctly for me :-)

>
>>
>> >
>> > Do we guarantee that you will never be appending to a NULL list (first
>> ==
>> > NULL), I didn't see that check.
>>
>> Here's my thinking on this. The guarantee is that there is always a
>> root node -- we don't ever remove that.
>
> I guess I'm confused here.  I was talking about when we call it from
> fixup_properties with chipnodes as the list.  It looks like we could delete
> all of them.  Do I have it backward?

yes. We should only ever delete mainboard nodes, not chip nodes.

ron




More information about the coreboot mailing list