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

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

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

On Fri, Aug 05, 2022 at 06:39:44AM -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.

Oh... sorry, obviously I hadn't read these later patches when I
complained about the command not printing property values.

> 
> We do that by casting the void data to a string array and:
> 
> - check if the array finishes with a null character
> - check if all characters are printable

This won't handle the case of the "string list" several strings tacked
together, separated by their terminating \0 characters.

> 
> If both conditions are met, we'll consider it to be a string data type
> and print it accordingly. After this change, 'info fdt' is now able to
> print string properties. Here's an example with the ARM 'virt' machine:
> 
> (qemu) info fdt /chosen
> chosen {
>     stdout-path = '/pl011@9000000'
>     rng-seed;
>     kaslr-seed;
> }
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  softmmu/device_tree.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 3fb07b537f..8691c3ccc0 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -663,6 +663,24 @@ 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(const void *data, int size)
> +{
> +    const char *str = data;
> +    int i;
> +
> +    if (size <= 0 || str[size - 1] != '\0') {
> +        return false;
> +    }
> +
> +    for (i = 0; i < size - 1; i++) {
> +        if (!g_ascii_isprint(str[i])) {
> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  static void fdt_format_node(GString *buf, int node, int depth)
>  {
>      const struct fdt_property *prop = NULL;
> @@ -681,7 +699,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(prop->data, prop_size)) {
> +            g_string_append_printf(buf, "%*s%s = '%s'\n",
> +                                   padding, "", propname, prop->data);

If you're going for dts like output, I'd suggest going all the way.
That means \" instead of \' and a ';' at the end of the line.

> +        } 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 --]