Re: [PATCH 09/24] dtoc: Support reading a list of arguments

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 08/02/2022 21:49, Simon Glass wrote:
> It is helpful to support a string or stringlist containing a list of
> space-separated arguments, for example:
> 
>    args = "-n fred", "-a", "123";
> 
> This resolves to the list:
> 
>    -n fred -a 123

Would be clearer as ['-n', 'fred', '-a', '123']

> 
> which can be passed to a program as arguments.
> 
> Add a helper to do the required processing.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  tools/dtoc/fdt_util.py               | 12 ++++++++++++
>  tools/dtoc/test/dtoc_test_simple.dts |  1 +
>  tools/dtoc/test_fdt.py               | 15 +++++++++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py
> index 19eb13aef3..59e065884f 100644
> --- a/tools/dtoc/fdt_util.py
> +++ b/tools/dtoc/fdt_util.py
> @@ -184,6 +184,18 @@ def GetStringList(node, propname, default=None):
>          return [strval]
>      return value
>  
> +def GetArgs(node, propname):
> +    prop = node.props.get(propname)
> +    if not prop:
> +        raise ValueError(f"Node '{node.path}': Expected property '{propname}'")
> +    if prop.bytes:
> +        value = GetStringList(node, propname)
> +    else:
> +        value = []

Isn't GetStringList(node, propname, default=[]) enough here, why check
prop.bytes?

> +    lists = [v.split() for v in value]

Use shlex.split() to handle quotes inside the strings, so that we can
pass args with spaces inside them. e.g. mkimage -n "U-Boot v2022.04".
Or each list element could be a single argument with no splitting done.

I also wish mkimage -n "U-Boot $(UBOOTRELEASE) for $(BOARD) board" from
Makefile was possible, but can't think of a great way.

> +    args = [x for l in lists for x in l]
> +    return args
> +

Anyway, I don't think this belongs here as argument lists are not really
a device-tree construct. It would be better in a new binman entry type
("command"?) which mkimage can subclass from.

>  def GetBool(node, propname, default=False):
>      """Get an boolean from a property
>  
> diff --git a/tools/dtoc/test/dtoc_test_simple.dts b/tools/dtoc/test/dtoc_test_simple.dts
> index 4c2c70af22..2d321fb034 100644
> --- a/tools/dtoc/test/dtoc_test_simple.dts
> +++ b/tools/dtoc/test/dtoc_test_simple.dts
> @@ -62,5 +62,6 @@
>  
>  	orig-node {
>  		orig = <1 23 4>;
> +		args = "-n first", "second", "-p", "123,456", "-x";

Could be useful to add an argument with single quotes, and one with
escaped double quotes.

>  	};
>  };
> diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
> index c8fe5fc1de..5d46e69b8b 100755
> --- a/tools/dtoc/test_fdt.py
> +++ b/tools/dtoc/test_fdt.py
> @@ -652,6 +652,21 @@ class TestFdtUtil(unittest.TestCase):
>          self.assertEqual(['test'],
>                           fdt_util.GetStringList(self.node, 'missing', ['test']))
>  
> +    def testGetArgs(self):
> +        node = self.dtb.GetNode('/orig-node')
> +        self.assertEqual(['message'], fdt_util.GetArgs(self.node, 'stringval'))
> +        self.assertEqual(
> +            ['multi-word', 'message'],
> +            fdt_util.GetArgs(self.node, 'stringarray'))
> +        self.assertEqual([], fdt_util.GetArgs(self.node, 'boolval'))
> +        self.assertEqual(['-n', 'first', 'second', '-p', '123,456', '-x'],
> +                         fdt_util.GetArgs(node, 'args'))
> +        with self.assertRaises(ValueError) as exc:
> +            fdt_util.GetArgs(self.node, 'missing')
> +        self.assertIn(
> +            "Node '/spl-test': Expected property 'missing'",
> +            str(exc.exception))
> +
>      def testGetBool(self):
>          self.assertEqual(True, fdt_util.GetBool(self.node, 'boolval'))
>          self.assertEqual(False, fdt_util.GetBool(self.node, 'missing'))