Nul string list v2
cover Nul string list v2
- 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 1402936552-10249-1-git-send-email-jack-jZyo8ZIaZD9AfugRpC6u6w@public.gmane.org
PATCH 1/2 Properly handle nul delimited string lists
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(-)
data.c +1 -1
@@ -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) {
Signature · 6 lines
2.0.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/2 Add embedded nul string list test
In reply to a message not in the archive--- 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
tests/nul_string_list.dtb new file binary +0 -0
tests/nul_string_list.dts new file binary +0 -0
tests/run_tests.sh +3 -0
@@ -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
Signature · 6 lines
2.0.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 1402936552-10249-3-git-send-email-jack-jZyo8ZIaZD9AfugRpC6u6w@public.gmane.org
Re: [PATCH 2/2] Add embedded nul string list test
Quoted 35 lines · 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.dtstests/nul_string_list.dtb new file binary +0
-0tests/nul_string_list.dts new file binary +0
-0tests/run_tests.sh +0
-0
tests/run_tests.sh dtc_tests () { · line 313
Quoted 4 lines
tests/run_tests.sh +1
-0@@ -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.
Quoted 5 lines
tests/run_tests.sh +2
-0+ 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
Signature · 4 lines
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [PATCH 2/2] Add embedded nul string list test
Quoted 8 lines · David Gibson
tests/run_tests.sh dtc_tests () { · line 313
Quoted 5 lines
tests/run_tests.sh +1
-0@@ -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.dtsdtb 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20140618173519.GA15740-O8SCTCEbm15XsEFxtoW7CMxtgHpCUUYS@public.gmane.org
Re: [PATCH 2/2] Add embedded nul string list test
Quoted 10 lines · Jack Miller
Quoted 8 lines · David Gibson
tests/run_tests.sh dtc_tests () { · line 313
Quoted 5 lines
tests/run_tests.sh +1
-0@@ -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.dtsdtb files generated by the tests should be named XXX.test.dtb so that make clean will clean them up again.
Quoted 6 lines
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.
Signature · 4 lines
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
Quoted 8 lines · Jack Miller
- Tweaked patch description to make failing case clearer
- Added testI 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.
Signature · 4 lines
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