Re: [PATCH for-7.2 v3 16/20] device_tree.c: support string array prop in fdt_format_node()

2022-08-17 Thread David Gibson
On Tue, Aug 16, 2022 at 02:34:24PM -0300, Daniel Henrique Barboza wrote:
> To support printing string properties in 'info fdt' we need to determine
> whether a void data might contain a string array.
> 
> We do that by casting the void data to a string array and:
> 
> - check if the array finishes with a null character
> - check if there's no empty string in the middle of the array (i.e.
> consecutive \0\0 characters)
> - check if all characters of each substring are printable
> 
> If all conditions are met, we'll consider it to be a string array data
> type and print it accordingly. After this change, 'info fdt' is now able
> to print string arrays. Here's an example of string arrays we're able to
> print in the /rtas node of the ppc64 pSeries machine:
> 
> (qemu) info fdt /rtas
> rtas {
> (...)
> qemu,hypertas-functions = "hcall-memop1";
> ibm,hypertas-functions = "hcall-pft","hcall-term","hcall-dabr",
> "hcall-interrupt","hcall-tce","hcall-vio","hcall-splpar","hcall-join",
> "hcall-bulk","hcall-set-mode","hcall-sprg0","hcall-copy","hcall-debug",
> "hcall-vphn","hcall-multi-tce","hcall-hpt-resize","hcall-watchdog";

nit: typical dts style would have extra spaces:
prop = "foo", "bar";
i.e. separated by ", " not just ",".

> }
> 
> 'qemu,hypertas-functions' is a property with a single string while
> 'ibm,hypertas-functions' is a string array.
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  softmmu/device_tree.c | 64 ++-
>  1 file changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 3fb07b537f..d32d6856da 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -663,6 +663,63 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error 
> **errp)
>  error_setg(errp, "Error when saving machine FDT to file %s", filename);
>  }
>  
> +static bool fdt_prop_is_string_array(const void *data, int size)
> +{
> +const char *str_arr, *str;
> +int i, str_len;
> +
> +str_arr = str = data;
> +
> +if (size <= 0 || str_arr[size - 1] != '\0') {
> +return false;
> +}
> +
> +while (str < str_arr + size) {
> +str_len = strlen(str);
> +
> +/*
> + * Do not consider empty strings (consecutives \0\0)
> + * as valid.
> + */
> +if (str_len == 0) {
> +return false;
> +}
> +
> +for (i = 0; i < str_len; i++) {
> +if (!g_ascii_isprint(str[i])) {
> +return false;
> +}
> +}
> +
> +str += str_len + 1;
> +}
> +
> +return true;
> +}
> +
> +static void fdt_prop_format_string_array(GString *buf,
> + const char *propname,
> + const char *data,
> + int prop_size, int padding)
> +{
> +const char *str = data;
> +
> +g_string_append_printf(buf, "%*s%s = ", padding, "", propname);
> +
> +while (str < data + prop_size) {
> +/* appends up to the next '\0' */
> +g_string_append_printf(buf, "\"%s\"", str);
> +
> +str += strlen(str) + 1;
> +if (str < data + prop_size) {
> +/* add a comma separator for the next string */
> +g_string_append_printf(buf, ",");
> +}
> +}
> +
> +g_string_append_printf(buf, ";\n");
> +}
> +
>  static void fdt_format_node(GString *buf, int node, int depth)
>  {
>  const struct fdt_property *prop = NULL;
> @@ -681,7 +738,12 @@ static void fdt_format_node(GString *buf, int node, int 
> depth)
>  prop = fdt_get_property_by_offset(fdt, property, &prop_size);
>  propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
>  
> -g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
> +if (fdt_prop_is_string_array(prop->data, prop_size)) {
> +fdt_prop_format_string_array(buf, propname, prop->data,
> + prop_size, padding);
> +} else {
> +g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
> +}
>  }
>  
>  padding -= 4;

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


signature.asc
Description: PGP signature


[PATCH for-7.2 v3 16/20] device_tree.c: support string array prop in fdt_format_node()

2022-08-16 Thread Daniel Henrique Barboza
To support printing string properties in 'info fdt' we need to determine
whether a void data might contain a string array.

We do that by casting the void data to a string array and:

- check if the array finishes with a null character
- check if there's no empty string in the middle of the array (i.e.
consecutive \0\0 characters)
- check if all characters of each substring are printable

If all conditions are met, we'll consider it to be a string array data
type and print it accordingly. After this change, 'info fdt' is now able
to print string arrays. Here's an example of string arrays we're able to
print in the /rtas node of the ppc64 pSeries machine:

(qemu) info fdt /rtas
rtas {
(...)
qemu,hypertas-functions = "hcall-memop1";
ibm,hypertas-functions = "hcall-pft","hcall-term","hcall-dabr",
"hcall-interrupt","hcall-tce","hcall-vio","hcall-splpar","hcall-join",
"hcall-bulk","hcall-set-mode","hcall-sprg0","hcall-copy","hcall-debug",
"hcall-vphn","hcall-multi-tce","hcall-hpt-resize","hcall-watchdog";
}

'qemu,hypertas-functions' is a property with a single string while
'ibm,hypertas-functions' is a string array.

Signed-off-by: Daniel Henrique Barboza 
---
 softmmu/device_tree.c | 64 ++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 3fb07b537f..d32d6856da 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -663,6 +663,63 @@ void qemu_fdt_qmp_dumpdtb(const char *filename, Error 
**errp)
 error_setg(errp, "Error when saving machine FDT to file %s", filename);
 }
 
+static bool fdt_prop_is_string_array(const void *data, int size)
+{
+const char *str_arr, *str;
+int i, str_len;
+
+str_arr = str = data;
+
+if (size <= 0 || str_arr[size - 1] != '\0') {
+return false;
+}
+
+while (str < str_arr + size) {
+str_len = strlen(str);
+
+/*
+ * Do not consider empty strings (consecutives \0\0)
+ * as valid.
+ */
+if (str_len == 0) {
+return false;
+}
+
+for (i = 0; i < str_len; i++) {
+if (!g_ascii_isprint(str[i])) {
+return false;
+}
+}
+
+str += str_len + 1;
+}
+
+return true;
+}
+
+static void fdt_prop_format_string_array(GString *buf,
+ const char *propname,
+ const char *data,
+ int prop_size, int padding)
+{
+const char *str = data;
+
+g_string_append_printf(buf, "%*s%s = ", padding, "", propname);
+
+while (str < data + prop_size) {
+/* appends up to the next '\0' */
+g_string_append_printf(buf, "\"%s\"", str);
+
+str += strlen(str) + 1;
+if (str < data + prop_size) {
+/* add a comma separator for the next string */
+g_string_append_printf(buf, ",");
+}
+}
+
+g_string_append_printf(buf, ";\n");
+}
+
 static void fdt_format_node(GString *buf, int node, int depth)
 {
 const struct fdt_property *prop = NULL;
@@ -681,7 +738,12 @@ static void fdt_format_node(GString *buf, int node, int 
depth)
 prop = fdt_get_property_by_offset(fdt, property, &prop_size);
 propname = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
 
-g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+if (fdt_prop_is_string_array(prop->data, prop_size)) {
+fdt_prop_format_string_array(buf, propname, prop->data,
+ prop_size, padding);
+} else {
+g_string_append_printf(buf, "%*s%s;\n", padding, "", propname);
+}
 }
 
 padding -= 4;
-- 
2.37.2