Commit 099902fc authored by Asbjørn Sloth Tønnesen's avatar Asbjørn Sloth Tønnesen Committed by Jakub Kicinski
Browse files

tools: ynl-gen: avoid repetitive variables definitions



In the generated attribute parsing code, avoid repetitively
defining the same variables over and over again, local to
the conditional block for each attribute.

This patch consolidates the definitions of local variables
for attribute parsing, so that they are defined at the
function level, and re-used across attributes, thus making
the generated code read more natural.

If attributes defines identical local_vars, then they will
be deduplicated, attributes are assumed to only use their
local variables transiently.

The example below shows how `len` was defined repeatedly in
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;
}

This patch results in this diffstat for the generated code:

$ diff -Naur pre/ post/ | diffstat
  devlink-user.c      |  187 +++----------------
  dpll-user.c         |   10 -
  ethtool-user.c      |   49 +----
  fou-user.c          |    5
  handshake-user.c    |    3
  mptcp_pm-user.c     |    3
  nfsd-user.c         |   16 -
  nl80211-user.c      |  159 +---------------
  nlctrl-user.c       |   21 --
  ovpn-user.c         |    7
  ovs_datapath-user.c |    9
  ovs_flow-user.c     |   89 ---------
  ovs_vport-user.c    |    7
  rt-addr-user.c      |   14 -
  rt-link-user.c      |  183 ++----------------
  rt-neigh-user.c     |   14 -
  rt-route-user.c     |   26 --
  rt-rule-user.c      |   11 -
  tc-user.c           |  380 +++++----------------------------------
  tcp_metrics-user.c  |    7
  team-user.c         |    5
  21 files changed, 175 insertions(+), 1030 deletions(-)

The changed lines are mostly `unsigned int len;` definitions:

$ diff -Naur pre/ post/ | grep ^[-+] | grep -v '^[-+]\{3\}' |
  grep -v '^.$' | sed -e 's/\t\+/ /g' | sort | uniq -c | sort -nr
    488 - unsigned int len;
    153 + unsigned int len;
     24 - const struct nlattr *attr2;
     18 + const struct nlattr *attr2;
      1 - __u32 policy_id, attr_id;
      1 + __u32 policy_id, attr_id;
      1 - __u32 op_id;
      1 + __u32 op_id;
      1 - const struct nlattr *attr_policy_id, *attr_attr_id;
      1 + const struct nlattr *attr_policy_id, *attr_attr_id;
      1 - const struct nlattr *attr_op_id;
      1 + const struct nlattr *attr_op_id;

Suggested-by: default avatarJakub Kicinski <kuba@kernel.org>
Signed-off-by: default avatarAsbjørn Sloth Tønnesen <ast@fiberby.net>
Reviewed-by: default avatarJakub Kicinski <kuba@kernel.org>
Link: https://patch.msgid.link/20250915144301.725949-6-ast@fiberby.net


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent db4ea3ba
Loading
Loading
Loading
Loading
+9 −5
Original line number Diff line number Diff line
@@ -242,7 +242,7 @@ class Type(SpecAttr):
        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:
@@ -250,10 +250,6 @@ class Type(SpecAttr):

        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))")
@@ -2113,6 +2109,7 @@ def _multi_parse(ri, struct, init_lines, local_vars):
            else:
                raise Exception("Per-op fixed header not supported, yet")

    var_set = set()
    array_nests = set()
    multi_attrs = set()
    needs_parg = False
@@ -2130,6 +2127,13 @@ def _multi_parse(ri, struct, init_lines, local_vars):
            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 Exception:
            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: