[coreboot] dt compiler patch

Myles Watson mylesgw at gmail.com
Tue Nov 25 17:19:45 CET 2008


> >> +/* 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.

> 
> >
> > 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?

thanks,
Myles

 
> New diff attached, with issue from your next email managed as well.
> 
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>





More information about the coreboot mailing list