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

[David Gibson <david@gibson.dropbear.id.au>]

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

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 <danielhb413@gmail.com>
> ---
>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]