Re: [PATCH 4/4] Create a new property value that means 'undefined'.

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.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