[libvirt PATCH] esx: generator: fix free of elements in lists

[Neal Gompa]

On Thu, Sep 10, 2020 at 7:48 AM Pino Toscano <ptoscano at redhat.com> wrote:
>
> When a list is freed, we iterate through all the items, invoking the
> free function for each; the actual free function called for each element
> is the function of the actual type of each element, and thus the @_next
> pointer in the element struct has the same type as the element itself.
> Currently, the free function gets the parent of the current element
> type, and invoke its free function to continue freeing the list.
> However, in case the hierarchy of the classes has more than 1 level
> (i.e. Class <- SubClass <- SubSubClass), the invoked free function is
> only the parent class' one, and not the actual base class of the
> hierarchy.
>
> To fix that, change the generator to get the ancestor of a class, and
> invoking that instead.  Also, avoid to set the @_next back, as it is not
> needed.
>
> Fixes commits 5cff36e39ae691fbd7c40597df1732eecf294150 and
> f76c6dde2e33233566e886d96e76b5fe0c102d9a.
>
> Signed-off-by: Pino Toscano <ptoscano at redhat.com>
> ---
>  scripts/esx_vi_generator.py | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/esx_vi_generator.py b/scripts/esx_vi_generator.py
> index 863c8af964..be9b3949e8 100755
> --- a/scripts/esx_vi_generator.py
> +++ b/scripts/esx_vi_generator.py
> @@ -751,13 +751,13 @@ class Object(GenericObject):
>          source += "{\n"
>
>          if self.features & Object.FEATURE__LIST:
> -            if self.extends is not None:
> +            ancestor = get_ancestor(self)
> +            if ancestor:
>                  # avoid "dereferencing type-punned pointer will break
>                  # strict-aliasing rules" warnings
> -                source += "    esxVI_%s *next = (esxVI_%s *)item->_next;\n\n" \
> -                          % (self.extends, self.extends)
> -                source += "    esxVI_%s_Free(&next);\n" % self.extends
> -                source += "    item->_next = (esxVI_%s *)next;\n\n" % self.name
> +                source += "    esxVI_%s *baseNext = (esxVI_%s *)item->_next;\n" \
> +                          % (ancestor, ancestor)
> +                source += "    esxVI_%s_Free(&baseNext);\n\n" % ancestor
>              else:
>                  source += "    esxVI_%s_Free(&item->_next);\n\n" % self.name
>
> @@ -1250,6 +1250,21 @@ def is_known_type(type):
>              type in enums_by_name)
>
>
> +def get_ancestor(obj):
> +    if not obj.extends:
> +        return None
> +    ancestor = None
> +    try:
> +        ancestor = ancestor_by_name[obj.extends]
> +    except KeyError:
> +        parent = objects_by_name[obj.extends]
> +        ancestor = get_ancestor(parent)
> +        if not ancestor:
> +            ancestor = parent.name
> +        ancestor_by_name[name] = ancestor
> +    return ancestor
> +
> +
>  def open_file(filename):
>      return open(filename, "wt")
>
> @@ -1341,6 +1356,7 @@ managed_objects_by_name = {}
>  enums_by_name = {}
>  methods_by_name = {}
>  block = None
> +ancestor_by_name = {}
>
>
>  # parse input file
> --
> 2.26.2
>

LGTM.

Reviewed-by: Neal Gompa <ngompa13 at gmail.com>

-- 
真実はいつも一つ！/ Always, there's only one truth!