Commit ce6cb811 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

tools: ynl-gen: individually free previous values on double set 



When user calls request_attrA_set() multiple times (for the same
attribute), and attrA is of type which allocates memory -
we try to free the previously associated values. For array
types (including multi-attr) we have only freed the array,
but the array may have contained pointers.

Refactor the code generation for free attr and reuse the generated
lines in setters to flush out the previous state. Since setters
are static inlines in the header we need to add forward declarations
for the free helpers of pure nested structs. Track which types get
used by arrays and include the right forwad declarations.

At least ethtool string set and bit set would not be freed without
this. Tho, admittedly, overriding already set attribute twice is likely
a very very rare thing to do.

Fixes: be5bea1c ("net: add basic C code generators for Netlink")
Reviewed-by: default avatarDonald Hunter <donald.hunter@gmail.com>
Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20250414211851.602096-4-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent dfa464b4

tools/net/ynl/pyynl/ynl_gen_c.py

+45 −17
Original line number Diff line number Diff line
@@ -162,9 +162,15 @@ class Type(SpecAttr): 
    def free_needs_iter(self):

        return False



    def free(self, ri, var, ref):

    def _free_lines(self, ri, var, ref):

        if self.is_multi_val() or self.presence_type() == 'len':

            ri.cw.p(f'free({var}->{ref}{self.c_name});')

            return [f'free({var}->{ref}{self.c_name});']

        return []



    def free(self, ri, var, ref):

        lines = self._free_lines(ri, var, ref)

        for line in lines:

            ri.cw.p(line)



    def arg_member(self, ri):

        member = self._complex_member_type(ri)
@@ -263,6 +269,10 @@ class Type(SpecAttr): 
        var = "req"

        member = f"{var}->{'.'.join(ref)}"



        local_vars = []

        if self.free_needs_iter():

            local_vars += ['unsigned int i;']



        code = []

        presence = ''

        for i in range(0, len(ref)):
@@ -272,6 +282,10 @@ class Type(SpecAttr): 
            if i == len(ref) - 1 and self.presence_type() != 'bit':

                continue

            code.append(presence + ' = 1;')

        ref_path = '.'.join(ref[:-1])

        if ref_path:

            ref_path += '.'

        code += self._free_lines(ri, var, ref_path)

        code += self._setter_lines(ri, member, presence)



        func_name = f"{op_prefix(ri, direction, deref=deref)}_set_{'_'.join(ref)}"
@@ -279,7 +293,8 @@ class Type(SpecAttr): 
        alloc = bool([x for x in code if 'alloc(' in x])

        if free and not alloc:

            func_name = '__' + func_name

        ri.cw.write_func('static inline void', func_name, body=code,

        ri.cw.write_func('static inline void', func_name, local_vars=local_vars,

                         body=code,

                         args=[f'{type_name(ri, direction, deref=deref)} *{var}'] + self.arg_member(ri))
@@ -482,8 +497,7 @@ class TypeString(Type): 
               ['unsigned int len;']



    def _setter_lines(self, ri, member, presence):

        return [f"free({member});",

                f"{presence}_len = strlen({self.c_name});",

        return [f"{presence}_len = strlen({self.c_name});",

                f"{member} = malloc({presence}_len + 1);",

                f'memcpy({member}, {self.c_name}, {presence}_len);',

                f'{member}[{presence}_len] = 0;']
@@ -536,8 +550,7 @@ class TypeBinary(Type): 
               ['unsigned int len;']



    def _setter_lines(self, ri, member, presence):

        return [f"free({member});",

                f"{presence}_len = len;",

        return [f"{presence}_len = len;",

                f"{member} = malloc({presence}_len);",

                f'memcpy({member}, {self.c_name}, {presence}_len);']
@@ -574,12 +587,14 @@ class TypeNest(Type): 
    def _complex_member_type(self, ri):

        return self.nested_struct_type



    def free(self, ri, var, ref):

    def _free_lines(self, ri, var, ref):

        lines = []

        at = '&'

        if self.is_recursive_for_op(ri):

            at = ''

            ri.cw.p(f'if ({var}->{ref}{self.c_name})')

        ri.cw.p(f'{self.nested_render_name}_free({at}{var}->{ref}{self.c_name});')

            lines += [f'if ({var}->{ref}{self.c_name})']

        lines += [f'{self.nested_render_name}_free({at}{var}->{ref}{self.c_name});']

        return lines



    def _attr_typol(self):

        return f'.type = YNL_PT_NEST, .nest = &{self.nested_render_name}_nest, '
@@ -632,15 +647,19 @@ class TypeMultiAttr(Type): 
    def free_needs_iter(self):

        return 'type' not in self.attr or self.attr['type'] == 'nest'



    def free(self, ri, var, ref):

    def _free_lines(self, ri, var, ref):

        lines = []

        if self.attr['type'] in scalars:

            ri.cw.p(f"free({var}->{ref}{self.c_name});")

            lines += [f"free({var}->{ref}{self.c_name});"]

        elif 'type' not in self.attr or self.attr['type'] == 'nest':

            ri.cw.p(f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)")

            ri.cw.p(f'{self.nested_render_name}_free(&{var}->{ref}{self.c_name}[i]);')

            ri.cw.p(f"free({var}->{ref}{self.c_name});")

            lines += [

                f"for (i = 0; i < {var}->{ref}n_{self.c_name}; i++)",

                f'{self.nested_render_name}_free(&{var}->{ref}{self.c_name}[i]);',

                f"free({var}->{ref}{self.c_name});",

            ]

        else:

            raise Exception(f"Free of MultiAttr sub-type {self.attr['type']} not supported yet")

        return lines



    def _attr_policy(self, policy):

        return self.base_type._attr_policy(policy)
@@ -666,8 +685,7 @@ class TypeMultiAttr(Type): 
    def _setter_lines(self, ri, member, presence):

        # For multi-attr we have a count, not presence, hack up the presence

        presence = presence[:-(len('_present.') + len(self.c_name))] + "n_" + self.c_name

        return [f"free({member});",

                f"{member} = {self.c_name};",

        return [f"{member} = {self.c_name};",

                f"{presence} = n_{self.c_name};"]
@@ -755,6 +773,7 @@ class Struct: 
        self.request = False

        self.reply = False

        self.recursive = False

        self.in_multi_val = False  # used by a MultiAttr or and legacy arrays



        self.attr_list = []

        self.attrs = dict()
@@ -1122,6 +1141,10 @@ class Family(SpecFamily): 
                    if attr in rs_members['reply']:

                        self.pure_nested_structs[nested].reply = True



                if spec.is_multi_val():

                    child = self.pure_nested_structs.get(nested)

                    child.in_multi_val = True



        self._sort_pure_types()



        # Propagate the request / reply / recursive
@@ -1136,6 +1159,8 @@ class Family(SpecFamily): 
                            struct.child_nests.update(child.child_nests)

                        child.request |= struct.request

                        child.reply |= struct.reply

                        if spec.is_multi_val():

                            child.in_multi_val = True

                if attr_set in struct.child_nests:

                    struct.recursive = True
@@ -2958,6 +2983,9 @@ def main(): 
            for attr_set, struct in parsed.pure_nested_structs.items():

                ri = RenderInfo(cw, parsed, args.mode, "", "", attr_set)

                print_type_full(ri, struct)

                if struct.request and struct.in_multi_val:

                    free_rsp_nested_prototype(ri)

                    cw.nl()



            for op_name, op in parsed.ops.items():

                cw.p(f"/* ============== {op.enum_name} ============== */")