Re: [patch net-next v1 1/2] tools: ynl: Use dict of predefined Structs to decode scalar types

[Jakub Kicinski <kuba@kernel.org>]

On Sun, 21 May 2023 18:07:32 +0100 Donald Hunter wrote:
> Use a dict of predefined Struct() objects to decode scalar types in native,
> big or little endian format. This removes the repetitive code for the
> scalar variants and ensures all the signed variants are supported.

> @@ -115,17 +116,17 @@ class NlAttr:
>          return self.raw
>  
>      def as_c_array(self, type):
> -        format, _ = self.type_formats[type]
> -        return list({ x[0] for x in struct.iter_unpack(format, self.raw) })
> +        format = self.get_format(type)
> +        return list({ x[0] for x in format.iter_unpack(self.raw) })

I probably asked about this before, and maybe not the question 
for this series but - why list({ ... }) and not [...]?

>          else:
> -            raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')
> +            try:
> +                format = NlAttr.get_format(attr['type'], attr.byte_order)
> +                attr_payload = format.pack(int(value))
> +            except:
> +                raise Exception(f'Unknown type at {space} {name} {value} {attr["type"]}')

Could we do:

	elif attr["type"] in NlAttr.type_formats:

instead? Maybe my C brain treats exceptions as too exceptional..

> +            elif attr_spec["type"]:
> +                try:
> +                    decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order)
> +                except:
> +                    raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}')

Same here.

Nice cleanup!