[PATCH v2 0/4] Add capability to create property from old property

[PATCH v2 0/4] Add capability to create property from old property

[Ayush Singh]

Allow construction of new values for a property using old property
value.

This was originally suggested in /append-property/ Patch series [0] and
can be used to achieve the same results (and much more).

In practice, it looks as follows:

dts-v1/;

/ {
	str-prop = "0";
	int-prop = <0>;
};

/ {
	str-prop = /./, "1", /./;
	int-prop = /./, <1>, /./;
};

dts to source output with -T -T also works as expected:

/dts-v1/;

/ { /* base.dts:3:3-5:3, base.dts:7:3-9:3 */
        int-prop = <0x00 0x01>, <0x02 0x03>, <0x00 0x01>; /* base.dts:4:9-4:26, base.dts:8:9-8:36 */
}; /* base.dts:3:3-5:3, base.dts:7:3-9:3 */

[0]: https://lore.kernel.org/devicetree-compiler/Z24nldCpXpoT7RaK@zatzit/T/#t

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
Changes in v2:
- Compare against expected dtb instead of checking each property
  individually in tests.
- Use xstrdup when copying refs so each marker has it's own copy.
- Pass struct data by value, not by pointer.
- Add srcpos_free helper to properly free srcpos which have been
  extended previously.
- Remove open items since my original choices seem correct.
- Link to v1: https://lore.kernel.org/r/20250301-previous-value-v1-0-71d612eb0ea9@beagleboard.org

---
Ayush Singh (4):
      Add alloc_marker
      srcpos: Define srcpos_free
      dtc: Add /./
      tests: Add test for /./

 data.c                       | 20 +++++++++++++-----
 dtc-lexer.l                  |  5 +++++
 dtc-parser.y                 |  5 +++++
 dtc.h                        |  3 +++
 livetree.c                   | 50 ++++++++++++++++++++++++++++++++++++++++++--
 srcpos.c                     | 11 ++++++++++
 srcpos.h                     |  1 +
 tests/prev_prop.dts          | 25 ++++++++++++++++++++++
 tests/prev_prop_expected.dts | 13 ++++++++++++
 tests/run_tests.sh           |  5 +++++
 10 files changed, 131 insertions(+), 7 deletions(-)
---
base-commit: ce1d8588880aecd7af264e422a16a8b33617cef7
change-id: 20241227-previous-value-c22d4eeaae72

Best regards,
-- 
Ayush Singh <ayush@beagleboard.org>

[PATCH v2 1/4] Add alloc_marker

[Ayush Singh]

- Add helper to allocate new marker

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 data.c | 20 +++++++++++++++-----
 dtc.h  |  2 ++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/data.c b/data.c
index 14734233ad8b7ebd38c3e62442b81aae66601806..913796f2e664d07cdc48e0cbf2ab5d6fe9978072 100644
--- a/data.c
+++ b/data.c
@@ -228,11 +228,7 @@ struct data data_add_marker(struct data d, enum markertype type, char *ref)
 {
 	struct marker *m;
 
-	m = xmalloc(sizeof(*m));
-	m->offset = d.len;
-	m->type = type;
-	m->ref = ref;
-	m->next = NULL;
+	m = alloc_marker(d.len, type, ref);
 
 	return data_append_markers(d, m);
 }
@@ -254,3 +250,17 @@ bool data_is_one_string(struct data d)
 
 	return true;
 }
+
+struct marker *alloc_marker(unsigned int offset, enum markertype type,
+			    char *ref)
+{
+	struct marker *m;
+
+	m = xmalloc(sizeof(*m));
+	m->offset = offset;
+	m->type = type;
+	m->ref = ref;
+	m->next = NULL;
+
+	return m;
+}
diff --git a/dtc.h b/dtc.h
index 4c4aaca1fc417c9d93e904e64b2c40216ee1b093..86928e1eea9764fe5d74d6dbb987589d65d54b66 100644
--- a/dtc.h
+++ b/dtc.h
@@ -183,6 +183,8 @@ struct data data_append_byte(struct data d, uint8_t byte);
 struct data data_append_zeroes(struct data d, int len);
 struct data data_append_align(struct data d, int align);
 
+struct marker *alloc_marker(unsigned int offset, enum markertype type,
+			    char *ref);
 struct data data_add_marker(struct data d, enum markertype type, char *ref);
 
 bool data_is_one_string(struct data d);

-- 
2.48.1

[PATCH v2 2/4] srcpos: Define srcpos_free

[Ayush Singh]

srcpos can be chained together using srcpos_extend. However, in such
cases, we need to free all the chained nodes.

srcpos_free is a helper to recursively free all the linked srcpos.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 srcpos.c | 11 +++++++++++
 srcpos.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/srcpos.c b/srcpos.c
index 5e2f7dd299184ff86b00b280dc31498cb9830e28..5bb57bf6856c6ff6b8eb993aff5d216c63bf5ecb 100644
--- a/srcpos.c
+++ b/srcpos.c
@@ -287,6 +287,17 @@ struct srcpos *srcpos_extend(struct srcpos *pos, struct srcpos *newtail)
 	return pos;
 }
 
+void srcpos_free(struct srcpos *pos)
+{
+	struct srcpos *p_next;
+
+	while (pos) {
+		p_next = pos->next;
+		free(pos);
+		pos = p_next;
+	}
+}
+
 char *
 srcpos_string(struct srcpos *pos)
 {
diff --git a/srcpos.h b/srcpos.h
index 4318d7ad34d91d2ada1a5d7f92d2c84148fec366..4d60b50e31197c2da6a7fa8851c5e5a6717d78a6 100644
--- a/srcpos.h
+++ b/srcpos.h
@@ -88,6 +88,7 @@ extern void srcpos_update(struct srcpos *pos, const char *text, int len);
 extern struct srcpos *srcpos_copy(struct srcpos *pos);
 extern struct srcpos *srcpos_extend(struct srcpos *new_srcpos,
 				    struct srcpos *old_srcpos);
+extern void srcpos_free(struct srcpos *pos);
 extern char *srcpos_string(struct srcpos *pos);
 extern char *srcpos_string_first(struct srcpos *pos, int level);
 extern char *srcpos_string_last(struct srcpos *pos, int level);

-- 
2.48.1

[PATCH v2 3/4] dtc: Add /./

[Ayush Singh]

Allow constructing new values for a property using old property values.
Can be used to append, pre-append, duplicate, etc.

In practice, it looks as follows:

dts-v1/;

/ {
	str-prop = "0";
	int-prop = <0>;
};

/ {
	str-prop = /./, "1", /./;
	int-prop = /./, <1>, /./;
};

dts to source output with -T -T also works as expected:

/dts-v1/;

/ { /* base.dts:3:3-5:3, base.dts:7:3-9:3 */
        int-prop = <0x00 0x01>, <0x02 0x03>, <0x00 0x01>; /* base.dts:4:9-4:26, base.dts:8:9-8:36 */
}; /* base.dts:3:3-5:3, base.dts:7:3-9:3 */

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 dtc-lexer.l  |  5 +++++
 dtc-parser.y |  5 +++++
 dtc.h        |  1 +
 livetree.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/dtc-lexer.l b/dtc-lexer.l
index de60a70b6bdbcb5ae4336ea4171ad6f645e91b36..5efeca10363e0c9c338b6578be9240c3f42249f0 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -144,6 +144,11 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
 			return DT_OMIT_NO_REF;
 		}
 
+<*>"/./" {
+			DPRINT("Keyword: /./\n");
+			return DT_PREV_PROP;
+		}
+
 <*>{LABEL}:	{
 			DPRINT("Label: %s\n", yytext);
 			yylval.labelref = xstrdup(yytext);
diff --git a/dtc-parser.y b/dtc-parser.y
index 4d5eece526243460203157464e3cd75f781e50e7..c34eb10a1068b5eb6a0f08e5a1db8066217b16bf 100644
--- a/dtc-parser.y
+++ b/dtc-parser.y
@@ -57,6 +57,7 @@ static bool is_ref_relative(const char *ref)
 %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
 %token DT_BITS
 %token DT_DEL_PROP
+%token DT_PREV_PROP
 %token DT_DEL_NODE
 %token DT_OMIT_NO_REF
 %token <propnodename> DT_PROPNODENAME
@@ -308,6 +309,10 @@ propdata:
 		{
 			$$ = data_merge($1, $2);
 		}
+        | propdataprefix DT_PREV_PROP
+                {
+                        $$ = data_add_marker($1, PREV_VALUE, NULL);
+                }
 	| propdataprefix arrayprefix '>'
 		{
 			$$ = data_merge($1, $2.data);
diff --git a/dtc.h b/dtc.h
index 86928e1eea9764fe5d74d6dbb987589d65d54b66..175fe3637a31cc28453dc27980f315a171763b49 100644
--- a/dtc.h
+++ b/dtc.h
@@ -110,6 +110,7 @@ enum markertype {
 	REF_PHANDLE,
 	REF_PATH,
 	LABEL,
+	PREV_VALUE,
 	TYPE_UINT8,
 	TYPE_UINT16,
 	TYPE_UINT32,
diff --git a/livetree.c b/livetree.c
index 93c77d95a320ec05aa355e12920cef9e1c91c26a..22d45ca90bec8971d02231787217deb7caf53310 100644
--- a/livetree.c
+++ b/livetree.c
@@ -139,10 +139,40 @@ struct node *reference_node(struct node *node)
 	return node;
 }
 
+static struct data data_insert_old_value(struct data d, struct marker *m,
+					 struct data old)
+{
+	unsigned int offset = m->offset;
+	struct marker *next = m->next;
+	struct marker *marker;
+	struct data new_data;
+	char *ref;
+
+	new_data = data_insert_at_marker(d, m, old.val, old.len);
+
+	/* Copy all markers from old value */
+	marker = old.markers;
+	for_each_marker(marker) {
+		ref = NULL;
+
+		if (marker->ref)
+			ref = xstrdup(marker->ref);
+
+		m->next = alloc_marker(marker->offset + offset, marker->type,
+				       ref);
+		m = m->next;
+	}
+	m->next = next;
+
+	return new_data;
+}
+
 struct node *merge_nodes(struct node *old_node, struct node *new_node)
 {
 	struct property *new_prop, *old_prop;
 	struct node *new_child, *old_child;
+	bool prev_value_used = false;
+	struct marker *marker;
 	struct label *l;
 
 	old_node->deleted = 0;
@@ -172,10 +202,26 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
 				for_each_label_withdel(new_prop->labels, l)
 					add_label(&old_prop->labels, l->label);
 
+				marker = new_prop->val.markers;
+				for_each_marker_of_type(marker, PREV_VALUE) {
+					new_prop->val = data_insert_old_value(
+						new_prop->val, marker,
+						old_prop->val);
+					prev_value_used = true;
+				}
+
 				old_prop->val = new_prop->val;
 				old_prop->deleted = 0;
-				free(old_prop->srcpos);
-				old_prop->srcpos = new_prop->srcpos;
+
+				if (prev_value_used) {
+					old_prop->srcpos =
+						srcpos_extend(old_prop->srcpos,
+							      new_prop->srcpos);
+				} else {
+					srcpos_free(old_prop->srcpos);
+					old_prop->srcpos = new_prop->srcpos;
+				}
+
 				free(new_prop);
 				new_prop = NULL;
 				break;

-- 
2.48.1

[PATCH v2 4/4] tests: Add test for /./

[Ayush Singh]

- Test /./ on a string and int array.
- Also test on subnode property.

Signed-off-by: Ayush Singh <ayush@beagleboard.org>
---
 tests/prev_prop.dts          | 25 +++++++++++++++++++++++++
 tests/prev_prop_expected.dts | 13 +++++++++++++
 tests/run_tests.sh           |  5 +++++
 3 files changed, 43 insertions(+)

diff --git a/tests/prev_prop.dts b/tests/prev_prop.dts
new file mode 100644
index 0000000000000000000000000000000000000000..27b5395628999b0a06ccd0c160d4e928bd6396ea
--- /dev/null
+++ b/tests/prev_prop.dts
@@ -0,0 +1,25 @@
+/dts-v1/;
+
+/ {
+        str-prop = "1", "2";
+        int-prop = <1 2>;
+
+        mixed-prop = "1", <0>;
+
+        subnode {
+                str-prop = "1", "2";
+                int-prop = <1 2>;
+        };
+};
+
+/ {
+        str-prop = /./, "3", "4", /./, "6";
+        int-prop = /./, <3 4>, /./, <6>;
+
+        mixed-prop = <1>, /./, "0";
+
+        subnode {
+                str-prop = /./, "3", "4", /./;
+                int-prop = /./, <3 4>, /./;
+        };
+};
diff --git a/tests/prev_prop_expected.dts b/tests/prev_prop_expected.dts
new file mode 100644
index 0000000000000000000000000000000000000000..59e6573674e55c3d7cf8b0f236324ec0f8585915
--- /dev/null
+++ b/tests/prev_prop_expected.dts
@@ -0,0 +1,13 @@
+/dts-v1/;
+
+/ {
+        str-prop = "1", "2", "3", "4", "1", "2", "6";
+        int-prop = <1 2 3 4 1 2 6>;
+
+        mixed-prop = <1>, "1", <0>, "0";
+
+        subnode {
+                str-prop = "1", "2", "3", "4", "1", "2";
+                int-prop = <1 2 3 4 1 2>;
+        };
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index f0b51c04bf0af69f1df483b185f3aefa5d0bae27..c66793aa0d99a8d066a675f70bdb33b3462d6fa8 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -708,6 +708,11 @@ dtc_tests () {
     run_dtc_test -I dts -O dtb -o dtc_tree1_delete.test.dtb "$SRCDIR/test_tree1_delete.dts"
     tree1_tests dtc_tree1_delete.test.dtb
 
+    # Check previous property functionality
+    run_dtc_test -I dts -O dtb -o prev_prop.test.dtb "$SRCDIR/prev_prop.dts"
+    run_dtc_test -I dts -O dtb -o prev_prop_expected.test.dtb "$SRCDIR/prev_prop_expected.dts"
+    run_test dtbs_equal_ordered prev_prop.test.dtb prev_prop_expected.test.dtb
+
     # Check omit-if-no-ref functionality
     run_dtc_test -I dts -O dtb -o omit-no-ref.test.dtb "$SRCDIR/omit-no-ref.dts"
     run_test check_path omit-no-ref.test.dtb not-exists "/node1"

-- 
2.48.1

Re: [PATCH v2 3/4] dtc: Add /./

[Andreas Gnau]

On 2025-03-11 16:35, Ayush Singh wrote:
 > Allow constructing new values for a property using old property values.
 > Can be used to append, pre-append, duplicate, etc.
 >
 > In practice, it looks as follows:
 >
 > dts-v1/;
 >
 > / {
 > 	str-prop = "0";
 > 	int-prop = <0>;
 > };
 >
 > / {
 > 	str-prop = /./, "1", /./;
 > 	int-prop = /./, <1>, /./;
 > };
 >
 > dts to source output with -T -T also works as expected:
 >
 > /dts-v1/;
 >
 > / { /* base.dts:3:3-5:3, base.dts:7:3-9:3 */
 >          int-prop = <0x00 0x01>, <0x02 0x03>, <0x00 0x01>; /* 
base.dts:4:9-4:26, base.dts:8:9-8:36 */
 > }; /* base.dts:3:3-5:3, base.dts:7:3-9:3 */

I think it will be a bit easier to understand if the two examples 
correspond to another, i.e. to have both int-prop and str-prop with the 
matching value from the first example in this example.

Re: [PATCH v2 1/4] Add alloc_marker

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2082 bytes --]

On Tue, Mar 11, 2025 at 09:05:37PM +0530, Ayush Singh wrote:
> - Add helper to allocate new marker
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

This is a reasonable cleanup regardless of anything else.  Applied.

> ---
>  data.c | 20 +++++++++++++++-----
>  dtc.h  |  2 ++
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/data.c b/data.c
> index 14734233ad8b7ebd38c3e62442b81aae66601806..913796f2e664d07cdc48e0cbf2ab5d6fe9978072 100644
> --- a/data.c
> +++ b/data.c
> @@ -228,11 +228,7 @@ struct data data_add_marker(struct data d, enum markertype type, char *ref)
>  {
>  	struct marker *m;
>  
> -	m = xmalloc(sizeof(*m));
> -	m->offset = d.len;
> -	m->type = type;
> -	m->ref = ref;
> -	m->next = NULL;
> +	m = alloc_marker(d.len, type, ref);
>  
>  	return data_append_markers(d, m);
>  }
> @@ -254,3 +250,17 @@ bool data_is_one_string(struct data d)
>  
>  	return true;
>  }
> +
> +struct marker *alloc_marker(unsigned int offset, enum markertype type,
> +			    char *ref)
> +{
> +	struct marker *m;
> +
> +	m = xmalloc(sizeof(*m));
> +	m->offset = offset;
> +	m->type = type;
> +	m->ref = ref;
> +	m->next = NULL;
> +
> +	return m;
> +}
> diff --git a/dtc.h b/dtc.h
> index 4c4aaca1fc417c9d93e904e64b2c40216ee1b093..86928e1eea9764fe5d74d6dbb987589d65d54b66 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -183,6 +183,8 @@ struct data data_append_byte(struct data d, uint8_t byte);
>  struct data data_append_zeroes(struct data d, int len);
>  struct data data_append_align(struct data d, int align);
>  
> +struct marker *alloc_marker(unsigned int offset, enum markertype type,
> +			    char *ref);
>  struct data data_add_marker(struct data d, enum markertype type, char *ref);
>  
>  bool data_is_one_string(struct data d);
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 2/4] srcpos: Define srcpos_free

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2045 bytes --]

On Tue, Mar 11, 2025 at 09:05:38PM +0530, Ayush Singh wrote:
> srcpos can be chained together using srcpos_extend. However, in such
> cases, we need to free all the chained nodes.
> 
> srcpos_free is a helper to recursively free all the linked srcpos.
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>

This should also _use_ the new function in any existing places it make
sense.  On a quick glance I can only see one such place, in
merge_nodes() where we use a plain free() on the srcpos info of an
overwritten property.

> ---
>  srcpos.c | 11 +++++++++++
>  srcpos.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/srcpos.c b/srcpos.c
> index 5e2f7dd299184ff86b00b280dc31498cb9830e28..5bb57bf6856c6ff6b8eb993aff5d216c63bf5ecb 100644
> --- a/srcpos.c
> +++ b/srcpos.c
> @@ -287,6 +287,17 @@ struct srcpos *srcpos_extend(struct srcpos *pos, struct srcpos *newtail)
>  	return pos;
>  }
>  
> +void srcpos_free(struct srcpos *pos)
> +{
> +	struct srcpos *p_next;
> +
> +	while (pos) {
> +		p_next = pos->next;
> +		free(pos);
> +		pos = p_next;
> +	}
> +}
> +
>  char *
>  srcpos_string(struct srcpos *pos)
>  {
> diff --git a/srcpos.h b/srcpos.h
> index 4318d7ad34d91d2ada1a5d7f92d2c84148fec366..4d60b50e31197c2da6a7fa8851c5e5a6717d78a6 100644
> --- a/srcpos.h
> +++ b/srcpos.h
> @@ -88,6 +88,7 @@ extern void srcpos_update(struct srcpos *pos, const char *text, int len);
>  extern struct srcpos *srcpos_copy(struct srcpos *pos);
>  extern struct srcpos *srcpos_extend(struct srcpos *new_srcpos,
>  				    struct srcpos *old_srcpos);
> +extern void srcpos_free(struct srcpos *pos);
>  extern char *srcpos_string(struct srcpos *pos);
>  extern char *srcpos_string_first(struct srcpos *pos, int level);
>  extern char *srcpos_string_last(struct srcpos *pos, int level);
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 3/4] dtc: Add /./

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5531 bytes --]

On Tue, Mar 11, 2025 at 09:05:39PM +0530, Ayush Singh wrote:
> Allow constructing new values for a property using old property values.
> Can be used to append, pre-append, duplicate, etc.
> 
> In practice, it looks as follows:
> 
> dts-v1/;
> 
> / {
> 	str-prop = "0";
> 	int-prop = <0>;
> };
> 
> / {
> 	str-prop = /./, "1", /./;
> 	int-prop = /./, <1>, /./;
> };
> 
> dts to source output with -T -T also works as expected:
> 
> /dts-v1/;
> 
> / { /* base.dts:3:3-5:3, base.dts:7:3-9:3 */
>         int-prop = <0x00 0x01>, <0x02 0x03>, <0x00 0x01>; /* base.dts:4:9-4:26, base.dts:8:9-8:36 */
> }; /* base.dts:3:3-5:3, base.dts:7:3-9:3 */
> 
> Signed-off-by: Ayush Singh <ayush@beagleboard.org>
> ---
>  dtc-lexer.l  |  5 +++++
>  dtc-parser.y |  5 +++++
>  dtc.h        |  1 +
>  livetree.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/dtc-lexer.l b/dtc-lexer.l
> index de60a70b6bdbcb5ae4336ea4171ad6f645e91b36..5efeca10363e0c9c338b6578be9240c3f42249f0 100644
> --- a/dtc-lexer.l
> +++ b/dtc-lexer.l
> @@ -144,6 +144,11 @@ static void PRINTF(1, 2) lexical_error(const char *fmt, ...);
>  			return DT_OMIT_NO_REF;
>  		}
>  
> +<*>"/./" {
> +			DPRINT("Keyword: /./\n");
> +			return DT_PREV_PROP;
> +		}
> +
>  <*>{LABEL}:	{
>  			DPRINT("Label: %s\n", yytext);
>  			yylval.labelref = xstrdup(yytext);
> diff --git a/dtc-parser.y b/dtc-parser.y
> index 4d5eece526243460203157464e3cd75f781e50e7..c34eb10a1068b5eb6a0f08e5a1db8066217b16bf 100644
> --- a/dtc-parser.y
> +++ b/dtc-parser.y
> @@ -57,6 +57,7 @@ static bool is_ref_relative(const char *ref)
>  %token DT_LSHIFT DT_RSHIFT DT_LE DT_GE DT_EQ DT_NE DT_AND DT_OR
>  %token DT_BITS
>  %token DT_DEL_PROP
> +%token DT_PREV_PROP

I think DT_PREV_VALUE would be a better name.  "prev prop" suggests
the property defined immediately above, rather than the previous value
of this property.

>  %token DT_DEL_NODE
>  %token DT_OMIT_NO_REF
>  %token <propnodename> DT_PROPNODENAME
> @@ -308,6 +309,10 @@ propdata:
>  		{
>  			$$ = data_merge($1, $2);
>  		}
> +        | propdataprefix DT_PREV_PROP
> +                {
> +                        $$ = data_add_marker($1, PREV_VALUE, NULL);
> +                }
>  	| propdataprefix arrayprefix '>'
>  		{
>  			$$ = data_merge($1, $2.data);
> diff --git a/dtc.h b/dtc.h
> index 86928e1eea9764fe5d74d6dbb987589d65d54b66..175fe3637a31cc28453dc27980f315a171763b49 100644
> --- a/dtc.h
> +++ b/dtc.h
> @@ -110,6 +110,7 @@ enum markertype {
>  	REF_PHANDLE,
>  	REF_PATH,
>  	LABEL,
> +	PREV_VALUE,
>  	TYPE_UINT8,
>  	TYPE_UINT16,
>  	TYPE_UINT32,
> diff --git a/livetree.c b/livetree.c
> index 93c77d95a320ec05aa355e12920cef9e1c91c26a..22d45ca90bec8971d02231787217deb7caf53310 100644
> --- a/livetree.c
> +++ b/livetree.c
> @@ -139,10 +139,40 @@ struct node *reference_node(struct node *node)
>  	return node;
>  }
>  
> +static struct data data_insert_old_value(struct data d, struct marker *m,
> +					 struct data old)

Nothing in this function requires 'old' to be the previous value of
the property.  So 'data_insert_data()' would be a better name.  It's
also strictly about manipulating a struct data so it should go in
data.c, not livetree.c.

> +{
> +	unsigned int offset = m->offset;
> +	struct marker *next = m->next;
> +	struct marker *marker;
> +	struct data new_data;
> +	char *ref;
> +
> +	new_data = data_insert_at_marker(d, m, old.val, old.len);
> +
> +	/* Copy all markers from old value */
> +	marker = old.markers;
> +	for_each_marker(marker) {
> +		ref = NULL;
> +
> +		if (marker->ref)
> +			ref = xstrdup(marker->ref);
> +
> +		m->next = alloc_marker(marker->offset + offset, marker->type,
> +				       ref);
> +		m = m->next;
> +	}
> +	m->next = next;
> +
> +	return new_data;
> +}
> +
>  struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  {
>  	struct property *new_prop, *old_prop;
>  	struct node *new_child, *old_child;
> +	bool prev_value_used = false;

This is logically per-property, not per-node.  So it needs to be
initialised to false for each property, not just once per node.  So it
might as well be local to the first while loop as well.

> +	struct marker *marker;
>  	struct label *l;
>  
>  	old_node->deleted = 0;
> @@ -172,10 +202,26 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>  				for_each_label_withdel(new_prop->labels, l)
>  					add_label(&old_prop->labels, l->label);
>  
> +				marker = new_prop->val.markers;
> +				for_each_marker_of_type(marker, PREV_VALUE) {
> +					new_prop->val = data_insert_old_value(
> +						new_prop->val, marker,
> +						old_prop->val);
> +					prev_value_used = true;
> +				}
> +
>  				old_prop->val = new_prop->val;
>  				old_prop->deleted = 0;
> -				free(old_prop->srcpos);
> -				old_prop->srcpos = new_prop->srcpos;
> +
> +				if (prev_value_used) {
> +					old_prop->srcpos =
> +						srcpos_extend(old_prop->srcpos,
> +							      new_prop->srcpos);
> +				} else {
> +					srcpos_free(old_prop->srcpos);
> +					old_prop->srcpos = new_prop->srcpos;
> +				}
> +
>  				free(new_prop);
>  				new_prop = NULL;
>  				break;
> 

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]