Jakub Kicinski <kuba@xxxxxxxxxx> writes:
> 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 [...]?
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.
>> 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.
>> + 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!
