[coreboot] dt compiler patch
Myles Watson
mylesgw at gmail.com
Tue Nov 25 17:54:18 CET 2008
On Tue, Nov 25, 2008 at 9:22 AM, ron minnich <rminnich at gmail.com> wrote:
> 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 :-)
I don't know if it's correct, but here's a first try:
/* we should only ever delete simple nodes: nodes with no kids. */
void del_node(struct node *node)
{
struct node *n, *s;
assert(! node->children);
if (first_node == node) {
first_node = node->next;
free(node);
return;
}
/* If the node is the first child, set parent->children link. */
if (node->parent->children == node)
node->parent->children = node->next_sibling;
else {
/* Remove from sibling list. */
for (n=node->parent->children; n->next_sibling;
n=n->next_sibling)
if (n->next_sibling == node)
break;
n->next_sibling = node->next_sibling;
}
for_all_nodes(n) {
if (n->next != node)
continue;
n->next = node->next;
if (node == last_node)
last_node = n;
break;
}
free(node);
}
It doesn't check to see if the child is really a child of its parent. I
figured that was a given.
> >>
> >> >
> >> > 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.
>
Right.
Thanks,
Myles
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081125/532ba5b2/attachment.html>
More information about the coreboot
mailing list