[PATCH 0/2] Nul string list v2

[PATCH 0/2] Nul string list v2

[Jack Miller]

- Tweaked patch description to make failing case clearer
- Added test

I decided to only test the mangled output since realloc was just a more serious
and obvious symptom of the same problem. I also verified that the pre-patch dtc
does in fact fail the test.

Let me know if there's anything else that needs to be done.

- Jack

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] Properly handle nul delimited string lists

[Jack Miller]

reserved-names="res1\0res2\0res3";

Where \0 is a true embedded nul in the DTS source, should properly turn
into a string list. To achieve this, use the len given by the lexer
instead of strlen.

Without this patch dtc will mangle the output and possible hang on
realloc.
---
 data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/data.c b/data.c
index 4c50b12..8cae237 100644
--- a/data.c
+++ b/data.c
@@ -74,7 +74,7 @@ struct data data_copy_escape_string(const char *s, int len)
 	struct data d;
 	char *q;
 
-	d = data_grow_for(empty_data, strlen(s)+1);
+	d = data_grow_for(empty_data, len + 1);
 
 	q = d.val;
 	while (i < len) {
-- 
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] Add embedded nul string list test

[Jack Miller]

---
 tests/nul_string_list.dtb | Bin 0 -> 127 bytes
 tests/nul_string_list.dts | Bin 0 -> 66 bytes
 tests/run_tests.sh        |   3 +++
 3 files changed, 3 insertions(+)
 create mode 100644 tests/nul_string_list.dtb
 create mode 100644 tests/nul_string_list.dts

diff --git a/tests/nul_string_list.dtb b/tests/nul_string_list.dtb
new file mode 100644
index 0000000000000000000000000000000000000000..ea2bc3915d8129c849d2cc7b247bc9d1a201256c
GIT binary patch
literal 127
zcmcb>`|m9S14BI!TL5tZ5NiOjAP@@xF$nMjF-Q&_FhT{GfwVM;U6fj!T2z*52xAz*
W7{*|gOhA$oswPD@FEKZ@m;nI8VH4E=

literal 0
HcmV?d00001

diff --git a/tests/nul_string_list.dts b/tests/nul_string_list.dts
new file mode 100644
index 0000000000000000000000000000000000000000..845a14425fd36db3fac2934d1412dbb2cd8fb3a9
GIT binary patch
literal 66
zcmdN-DJj-1Gt{@{;?h^B=He_$Elw>eOHI+uOUz9zR<Ko2f{GY2Kp93bhOv@0SFJS{
E0H@#-od5s;

literal 0
HcmV?d00001

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index f205ce6..3b0b500 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -310,6 +310,9 @@ dtc_tests () {
     run_dtc_test -I dts -O dtb -o dtc_comments-cmp.test.dtb comments-cmp.dts
     run_test dtbs_equal_ordered dtc_comments.test.dtb dtc_comments-cmp.test.dtb
 
+    run_dtc_test -I dts -O dtb -o dtc_nul_string_list.dtb nul_string_list.dts
+    run_test dtbs_equal_ordered dtc_nul_string_list.dtb nul_string_list.dtb
+
     # Check aliases support in fdt_path_offset
     run_dtc_test -I dts -O dtb -o aliases.dtb aliases.dts
     run_test get_alias aliases.dtb
-- 
2.0.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] Nul string list v2

[David Gibson]

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

On Mon, Jun 16, 2014 at 11:35:50AM -0500, Jack Miller wrote:
> - Tweaked patch description to make failing case clearer
> - Added test
> 
> I decided to only test the mangled output since realloc was just a more serious
> and obvious symptom of the same problem. I also verified that the pre-patch dtc
> does in fact fail the test.
> 
> Let me know if there's anything else that needs to be done.

Sure, couple of nits to pick.

It's preferred to fold fix and testcase into the same patch -
particularly for small fixes like this.  Further comment against the
individual patches.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] Add embedded nul string list test

[David Gibson]

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

On Mon, Jun 16, 2014 at 11:35:52AM -0500, Jack Miller wrote:
> ---
>  tests/nul_string_list.dtb | Bin 0 -> 127 bytes
>  tests/nul_string_list.dts | Bin 0 -> 66 bytes
>  tests/run_tests.sh        |   3 +++
>  3 files changed, 3 insertions(+)
>  create mode 100644 tests/nul_string_list.dtb
>  create mode 100644 tests/nul_string_list.dts
> 
> diff --git a/tests/nul_string_list.dtb b/tests/nul_string_list.dtb
> new file mode 100644
> index 0000000000000000000000000000000000000000..ea2bc3915d8129c849d2cc7b247bc9d1a201256c
> GIT binary patch
> literal 127
> zcmcb>`|m9S14BI!TL5tZ5NiOjAP@@xF$nMjF-Q&_FhT{GfwVM;U6fj!T2z*52xAz*
> W7{*|gOhA$oswPD@FEKZ@m;nI8VH4E=
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/nul_string_list.dts b/tests/nul_string_list.dts
> new file mode 100644
> index 0000000000000000000000000000000000000000..845a14425fd36db3fac2934d1412dbb2cd8fb3a9
> GIT binary patch
> literal 66
> zcmdN-DJj-1Gt{@{;?h^B=He_$Elw>eOHI+uOUz9zR<Ko2f{GY2Kp93bhOv@0SFJS{
> E0H@#-od5s;
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index f205ce6..3b0b500 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -310,6 +310,9 @@ dtc_tests () {
>      run_dtc_test -I dts -O dtb -o dtc_comments-cmp.test.dtb comments-cmp.dts
>      run_test dtbs_equal_ordered dtc_comments.test.dtb dtc_comments-cmp.test.dtb
>  
> +    run_dtc_test -I dts -O dtb -o dtc_nul_string_list.dtb nul_string_list.dts

dtb files generated by the tests should be named XXX.test.dtb so that
make clean will clean them up again.

> +    run_test dtbs_equal_ordered dtc_nul_string_list.dtb nul_string_list.dtb
> +
>      # Check aliases support in fdt_path_offset
>      run_dtc_test -I dts -O dtb -o aliases.dtb aliases.dts
>      run_test get_alias aliases.dtb

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] Add embedded nul string list test

[Jack Miller]

On Wed, Jun 18, 2014 at 11:27:29AM +1000, David Gibson wrote:
> > @@ -310,6 +310,9 @@ dtc_tests () {
> >      run_dtc_test -I dts -O dtb -o dtc_comments-cmp.test.dtb comments-cmp.dts
> >      run_test dtbs_equal_ordered dtc_comments.test.dtb dtc_comments-cmp.test.dtb
> >  
> > +    run_dtc_test -I dts -O dtb -o dtc_nul_string_list.dtb nul_string_list.dts
> 
> dtb files generated by the tests should be named XXX.test.dtb so that
> make clean will clean them up again.

Hmmm, the greater problem here seems to be that I'm including a .dtb that
*isn't* disposable and will be cleaned. Should this test check that the
generated dtb decompiles into "","","" format instead of comparing to a
binary? That seems a little weak as it assumes that the intermediate .dtb is
correct, but I don't know your thoughts on including binaries into your test
directory.

- Jack
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Add embedded nul string list test

[David Gibson]

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

On Wed, Jun 18, 2014 at 12:35:19PM -0500, Jack Miller wrote:
> On Wed, Jun 18, 2014 at 11:27:29AM +1000, David Gibson wrote:
> > > @@ -310,6 +310,9 @@ dtc_tests () {
> > >      run_dtc_test -I dts -O dtb -o dtc_comments-cmp.test.dtb comments-cmp.dts
> > >      run_test dtbs_equal_ordered dtc_comments.test.dtb dtc_comments-cmp.test.dtb
> > >  
> > > +    run_dtc_test -I dts -O dtb -o dtc_nul_string_list.dtb nul_string_list.dts
> > 
> > dtb files generated by the tests should be named XXX.test.dtb so that
> > make clean will clean them up again.
> 
> Hmmm, the greater problem here seems to be that I'm including a .dtb that
> *isn't* disposable and will be cleaned. Should this test check that the
> generated dtb decompiles into "","","" format instead of comparing to a
> binary? That seems a little weak as it assumes that the intermediate .dtb is
> correct, but I don't know your thoughts on including binaries into your test
> directory.

So, I thought about that.  It would be possible to avoid by making a
comparison .dts which includes the NULs using string escapes or
bytestrings instead of NULs in the input, then comparing the .dtb
outputs from each.

However, I don't think there's a real problem with a pre-built dtb,
git handles binary files adequately well.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]