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

Jakub Kicinski [off-list ref] writes:

On Sun, 21 May 2023 18:07:32 +0100 Donald Hunter wrote:

quoted

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.

quoted

@@ -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 [...]?

It looks like I cargo-culted something there, and it's just plain
wrong. Reading it now, it's clearly a set comprehension coerced into a
list, which could well change ordering.

I'll fix this in the next version.

quoted

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

Good suggestion, that's much cleaner.

quoted

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

Ack.

Nice cleanup!