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

quoted

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?

quoted

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.

quoted

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: