From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH 4/4] Create a new property value that means 'undefined'. Date: Thu, 21 Oct 2010 17:14:09 +1100 Message-ID: <20101021061409.GJ6227@yookeroo> References: <20101020214419.2985.51068.stgit@riker> <20101020214522.2985.97224.stgit@riker> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20101020214522.2985.97224.stgit@riker> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: John Bonesio Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org List-Id: devicetree@vger.kernel.org On Wed, Oct 20, 2010 at 02:45:22PM -0700, John Bonesio wrote: > When updating existing nodes in a device tree merge operation, properties > can be removed by setting the value to /undef-prop/. > > if /undef-prop/ is assigned to a property that doesn't exist, the property > is treated the same as if it had not been declared. [snip] > diff --git a/dtc.h b/dtc.h > index a7f3667..b3fca6e 100644 > --- a/dtc.h > +++ b/dtc.h > @@ -130,7 +130,12 @@ struct label { > struct label *next; > }; > > +#define PROP_DEFINED (0) > +#define PROP_UNDEFINED (1) You never actually use these constants. > + > struct property { > + int undefined; /* if the property is set to undefined, this feild is > + set to PROP_UNDEFINED */ > char *name; > struct data val; > > @@ -170,6 +175,7 @@ void add_label(struct label **labels, char *label); > struct property *build_property(char *name, struct data val); > struct property *chain_property(struct property *first, struct property *list); > struct property *reverse_properties(struct property *first); > +void undefine_property(struct property *prop); > > struct node *build_node(struct property *proplist, struct node *children); > struct node *name_node(struct node *node, char *name); > @@ -177,6 +183,7 @@ struct node *chain_node(struct node *first, struct node *list); > struct node *merge_nodes(struct node *old_node, struct node *new_node); > > void add_property(struct node *node, struct property *prop); > +void remove_property(struct node *node, struct property *prop); > void add_child(struct node *parent, struct node *child); > void remove_child(struct node *parent, struct node *child); > > diff --git a/flattree.c b/flattree.c > index ead0332..00439e9 100644 > --- a/flattree.c > +++ b/flattree.c > @@ -275,6 +275,9 @@ static void flatten_tree(struct node *tree, struct emitter *emit, > for_each_property(tree, prop) { > int nameoff; > > + if (prop->undefined) > + continue; > + > if (streq(prop->name, "name")) > seen_name_prop = 1; > > diff --git a/livetree.c b/livetree.c > index bf8796b..2ef734d 100644 > --- a/livetree.c > +++ b/livetree.c > @@ -51,6 +51,11 @@ struct property *build_property(char *name, struct data val) > return new; > } > > +void undefine_property(struct property *prop) > +{ > + prop->undefined = 1; > +} > + > struct property *chain_property(struct property *first, struct property *list) > { > assert(first->next == NULL); > @@ -121,11 +126,15 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node) > /* Look for a collision, set new value if there is */ > for_each_property(old_node, old_prop) { > if (streq(old_prop->name, new_prop->name)) { > - /* Add new labels to old property */ > - for_each_label(new_prop->labels, l) > - add_label(&old_prop->labels, l->label); > - > - old_prop->val = new_prop->val; > + if (new_prop->undefined) { > + remove_property(old_node, > old_prop); This will lose property labels into the ether as your earlier patch lost node labels. > + } else { > + /* Add new labels to old property */ > + for_each_label(new_prop->labels, l) > + add_label(&old_prop->labels, l->label); > + > + old_prop->val = new_prop->val; > + } > free(new_prop); > new_prop = NULL; > break; > @@ -188,6 +197,25 @@ void add_property(struct node *node, struct property *prop) > *p = prop; > } > > +void remove_property(struct node *node, struct property *prop) > +{ > + struct property **p; > + int found = 0; > + > + p = &node->proplist; > + while (*p) { > + if (*p == prop) { > + *p = (*p)->next; > + found = 1; > + break; > + } > + p = &((*p)->next); > + } > + /* property not in the node? it's probably an error, so flag it. */ > + assert(found); assert()s are for finding bugs in code, not detecting errors in user input. AFAICT this *can* be generated by usre input so it should print an error message and if possible continue, not cause a SIGABRT. > +} > + > + > void add_child(struct node *parent, struct node *child) > { > struct node **p; > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson