On Sat, 6 Sep 2025 13:13:29 +0000 Asbjørn Sloth Tønnesen wrote:

In patch 4, it is about a variable used by multiple Type classes having
presence_type() = 'count', which is currently 3 classes:
- TypeBinaryScalarArray
- TypeMultiAttr
- TypeArrayNest (later renamed to TypeIndexedArray)

In patch 5, I move code for a special variable used by one Type class,
to be contained within that class. It makes it easier to ensure that the
variable is only defined, when used, and vice versa. This comes at the
cost of the generated code looking generated.

So you're agreeing?

If we should make the generated code look like it was written by humans,
then I would move the definition of these local variables into a class
method, so `i` can be generated by the generic implementation, and `array`
can be implemented in it's class. I will take a stab at this, but it might
be too much refactoring for this series, eg. `len` is also defined local
to conditional blocks multiple branches in a row.

tools/net/ynl/generated/nl80211-user.c:
nl80211_iftype_data_attrs_parse(..) {
   [..]
   ynl_attr_for_each_nested(attr, nested) {
     unsigned int type = ynl_attr_type(attr);

     if (type == NL80211_BAND_IFTYPE_ATTR_IFTYPES) {
       unsigned int len;
       [..]
     } else if (type == NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC) {
       unsigned int len;
       [..]
     [same pattern 8 times, so 11 times in total]
     } else if (type == NL80211_BAND_IFTYPE_ATTR_EHT_CAP_PPE) {
       unsigned int len;
       [..]
     }
   }
   return 0;
}

It's pretty easily doable, I already gave up on not calling _attr_get()
for sub-messages.

That looks very generated, I would have `len` defined together with `type`,
and a switch statement would also look a lot more natural, but maybe leave
the if->switch conversion for the compiler to detect.

diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py
index fb7e03805a11..8a1f8a477566 100755
--- a/tools/net/ynl/pyynl/ynl_gen_c.py
+++ b/tools/net/ynl/pyynl/ynl_gen_c.py

@@ -243,7 +243,7 @@ from lib import SpecSubMessage, SpecSubMessageFormat
         raise Exception(f"Attr get not implemented for class type {self.type}")
 
     def attr_get(self, ri, var, first):
-        lines, init_lines, local_vars = self._attr_get(ri, var)
+        lines, init_lines, _ = self._attr_get(ri, var)
         if type(lines) is str:
             lines = [lines]
         if type(init_lines) is str:

@@ -251,10 +251,6 @@ from lib import SpecSubMessage, SpecSubMessageFormat
 
         kw = 'if' if first else 'else if'
         ri.cw.block_start(line=f"{kw} (type == {self.enum_name})")
-        if local_vars:
-            for local in local_vars:
-                ri.cw.p(local)
-            ri.cw.nl()
 
         if not self.is_multi_val():
             ri.cw.p("if (ynl_attr_validate(yarg, attr))")

@@ -2101,6 +2097,7 @@ _C_KW = {
             else:
                 raise Exception(f"Per-op fixed header not supported, yet")
 
+    var_set = set()
     array_nests = set()
     multi_attrs = set()
     needs_parg = False

@@ -2118,6 +2115,13 @@ _C_KW = {
             multi_attrs.add(arg)
         needs_parg |= 'nested-attributes' in aspec
         needs_parg |= 'sub-message' in aspec
+
+        try:
+            _, _, l_vars = aspec._attr_get(ri, '')
+            var_set |= set(l_vars) if l_vars else set()
+        except:
+            pass  # _attr_get() not implemented by simple types, ignore
+    local_vars += list(var_set)
     if array_nests or multi_attrs:
         local_vars.append('int i;')
     if needs_parg: