From: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
To: "Uwe Kleine-König"
<uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
entwicklung-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
"Uwe Kleine-König"
<u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Subject: Re: [PATCH v2] write_propval_string: Use a list of strings instead of "\0" in a string
Date: Sat, 29 Apr 2023 16:29:50 +1000 [thread overview]
Message-ID: <ZEy5Xnh/DM/wzUWa@yekko> (raw)
In-Reply-To: <20230428113217.744447-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]
On Fri, Apr 28, 2023 at 01:32:17PM +0200, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>
> A string that contains '\0' can be written as a list of strings e.g.
>
> clock-names = "di0_pll\0di1_pll\0di0_sel\0di1_sel\0di2_sel\0di3_sel\0di0\0di1";
>
> is equivalent to
>
> clock-names = "di0_pll", "di1_pll", "di0_sel", "di1_sel", "di2_sel", "di3_sel", "di0", "di1";
>
> The latter is easier to read, to use this format instead.
>
> Two test files are adapted accordingly to keep the test suite happy.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
> Changes since (implicit) v1, sent with Message-Id:
> 20230426182405.572729-1-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org:
>
> - Adapt the test suite
>
> tests/type-preservation.dt.yaml | 2 +-
> tests/type-preservation.dts | 2 +-
> treesource.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/type-preservation.dt.yaml b/tests/type-preservation.dt.yaml
> index a0cc64cc4b69..e238d395aa02 100644
> --- a/tests/type-preservation.dt.yaml
> +++ b/tests/type-preservation.dt.yaml
> @@ -12,7 +12,7 @@
> int16-matrix: [!u16 [0x1234, 0x5678], [0x90ab, 0xcdef]]
> int64: [!u64 [0x200000000]]
> int64-array: [!u64 [0x100000000, 0x0]]
> - a-string-with-nulls: ["foo\0bar", "baz"]
> + a-string-array: ["foo", "bar", "baz"]
Ah. I was afraid of this. So "fixing" the test highlights another
problem. It's pretty clear to me that the whole point of this test is
to test the preservation of the internal \0, as distinct from the
separate strings later in there. So, this is exercising an edge case
of the typing markers we now add.
Now.. personally, I've never been particularly convinced that the type
preservation stuff was a particularly good idea. It will never be
perfect, and it can give a misleading impression that information is
preserved into the dtb which isn't. But, it's pretty well established
now, and I assume people had reasons for wanting it.
So, your patch makes things nicer when going from dtb -> dts, but
breaks an established feature when going dts -> dts, or dts -> yaml.
So, I think we need to rethink.
Rather than changing how "strings" are emitted, I think you want to
change how we guess typing information when coming from dtb: instead
of just putting a single string marker on the property, we should put
one after every \0 and we should get the output you're after.
> a-phandle: [[!phandle 0x1]]
> a-phandle-with-args: [[!phandle 0x1, 0x0, 0x1], [!phandle 0x1, 0x2, 0x3]]
> subsubnode:
> diff --git a/tests/type-preservation.dts b/tests/type-preservation.dts
> index 921ea21172d1..443424dc28ec 100644
> --- a/tests/type-preservation.dts
> +++ b/tests/type-preservation.dts
> @@ -15,7 +15,7 @@
> int16-matrix = /bits/ 16 <0x1234 0x5678>, <0x90ab 0xcdef>;
> int64 = /bits/ 64 <0x200000000>;
> int64-array = /bits/ 64 <0x100000000 0x00> int64_array_label_end:;
> - a-string-with-nulls = "foo\0bar", "baz";
> + a-string-array = "foo", "bar", "baz";
> a-phandle = <&subsub1>;
> a-phandle-with-args = <&subsub1 0x00 0x01>, <&subsub1 0x02 0x03>;
>
> diff --git a/treesource.c b/treesource.c
> index 33fedee82d58..de30188189fb 100644
> --- a/treesource.c
> +++ b/treesource.c
> @@ -87,7 +87,7 @@ static void write_propval_string(FILE *f, const char *s, size_t len)
> fprintf(f, "\\\"");
> break;
> case '\0':
> - fprintf(f, "\\0");
> + fprintf(f, "\", \"");
> break;
> default:
> if (isprint((unsigned char)c))
--
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2023-04-29 6:29 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-28 11:32 [PATCH v2] write_propval_string: Use a list of strings instead of "\0" in a string Uwe Kleine-König
[not found] ` <20230428113217.744447-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
2023-04-29 6:29 ` David Gibson [this message]
2023-04-30 13:23 ` Uwe Kleine-König
2023-05-01 18:43 ` Rob Herring
[not found] ` <CAL_JsqJ9QManUTkOpgaE85b-uVB_JQZGsLXdJCz7_5J_3Aqq2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-05-03 13:43 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZEy5Xnh/DM/wzUWa@yekko \
--to=david-xt8fgy+axnrb3ne2bgzf6laj5h9x9tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=entwicklung-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
--cc=u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
--cc=uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).