| Message ID | 20191108194758.17813-2-palmer@dabbelt.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | device_tree: Allow for and use string arrays [Was: RISC-V: virt: This is a "sifive, test1" test finisher] | expand |
On Fri, 8 Nov 2019 at 19:48, Palmer Dabbelt <palmer@dabbelt.com> wrote: > > The device tree format allows for arrays of strings, which are encoded > with '\0's inside regular strings. These are ugly to represent in C, so > the helper function represents them as strings with internal '\0's that > are terminated with a double '\0'. In other words, the array > ["string1", "string2"] is represeted as "string1\0string2\0". > > The DTB generated by this function is accepted by DTC and produces an > array of strings, but I can't find any explicit line in the DT > specification that defines how these are encoded. > +/* > + * This uses a particularly odd encoding: "strings" is a list of strings that > + * must be terminated by two back-to-back '\0' characters. > + */ > +int qemu_fdt_setprop_strings(void *fdt, const char *node_path, > + const char *property, const char *strings); The clean API for this would be to use varargs so you could write qemu_fdt_setprop_stringlist(fdt, node, prop, "arm,armv8-timer", "arm,armv7-timer"); and have it do the assembly into the encoding that fdt expects. That would require us to do a bit of allocation-and-freeing to assemble the string, of course, but then we only do fdt creation at startup. NB: I think that this is a good idea but not-for-4.2 material, so if you wanted your sifive board change to go into 4.2 you should probably start with the simple approach and leave the refactoring for the next release cycle. thanks -- PMM
On Sat, Nov 09, 2019 at 03:59:24PM +0000, Peter Maydell wrote: > On Fri, 8 Nov 2019 at 19:48, Palmer Dabbelt <palmer@dabbelt.com> wrote: > > > > The device tree format allows for arrays of strings, which are encoded > > with '\0's inside regular strings. These are ugly to represent in C, so > > the helper function represents them as strings with internal '\0's that > > are terminated with a double '\0'. In other words, the array > > ["string1", "string2"] is represeted as "string1\0string2\0". > > > > The DTB generated by this function is accepted by DTC and produces an > > array of strings, but I can't find any explicit line in the DT > > specification that defines how these are encoded. > > > +/* > > + * This uses a particularly odd encoding: "strings" is a list of strings that > > + * must be terminated by two back-to-back '\0' characters. > > + */ > > +int qemu_fdt_setprop_strings(void *fdt, const char *node_path, > > + const char *property, const char *strings); > > The clean API for this would be to use varargs so you could write > > qemu_fdt_setprop_stringlist(fdt, node, prop, "arm,armv8-timer", > "arm,armv7-timer"); > > and have it do the assembly into the encoding that fdt expects. > That would require us to do a bit of allocation-and-freeing > to assemble the string, of course, but then we only do fdt > creation at startup. Right, I really don't see the value in this interface. Using "foo\0bar" is a little ugly, but not really any uglier than "foo\0bar\0". The existing interface would be a drag if you had dynamically created entries in the list (because getting the size can't be done with sizeof() then), but I don't think that's actually a very likely usecase. > NB: I think that this is a good idea but not-for-4.2 material, > so if you wanted your sifive board change to go into 4.2 you > should probably start with the simple approach and leave the > refactoring for the next release cycle. I concur.
diff --git a/device_tree.c b/device_tree.c index f8b46b3c73..b4379f13a7 100644 --- a/device_tree.c +++ b/device_tree.c @@ -397,6 +397,23 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, return r; } +static size_t stringarr_length(const char *strings) +{ + size_t count = 1; + while (strings[count - 1] != '\0' || strings[count] != '\0') { + count++; + } + return count; +} + +int qemu_fdt_setprop_strings(void *fdt, const char *node_path, + const char *property, const char *strings) +{ + size_t length = stringarr_length(strings); + return qemu_fdt_setprop(fdt, node_path, property, strings, length); +} + + const void *qemu_fdt_getprop(void *fdt, const char *node_path, const char *property, int *lenp, Error **errp) { diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index c16fd69bc0..d43c07128e 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -70,6 +70,12 @@ int qemu_fdt_setprop_string(void *fdt, const char *node_path, int qemu_fdt_setprop_phandle(void *fdt, const char *node_path, const char *property, const char *target_node_path); +/* + * This uses a particularly odd encoding: "strings" is a list of strings that + * must be terminated by two back-to-back '\0' characters. + */ +int qemu_fdt_setprop_strings(void *fdt, const char *node_path, + const char *property, const char *strings); /** * qemu_fdt_getprop: retrieve the value of a given property * @fdt: pointer to the device tree blob
The device tree format allows for arrays of strings, which are encoded with '\0's inside regular strings. These are ugly to represent in C, so the helper function represents them as strings with internal '\0's that are terminated with a double '\0'. In other words, the array ["string1", "string2"] is represeted as "string1\0string2\0". The DTB generated by this function is accepted by DTC and produces an array of strings, but I can't find any explicit line in the DT specification that defines how these are encoded. Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com> --- device_tree.c | 17 +++++++++++++++++ include/sysemu/device_tree.h | 6 ++++++ 2 files changed, 23 insertions(+)