Re: Fix memleak: temporary skyline objects for systems were never deleted (issue 4923048)

[David Kastrup]

address@hidden writes:

> Reviewers: ,
>
> Message:
> This patch fixes a memleak: Some temporary skylines were never
> deleted...
> Please review
>
> Description:
> Fix memleak: temporary skyline objects for systems were never deleted
>
> Please review this at http://codereview.appspot.com/4923048/
>
> Affected files:
>    M lily/skyline.cc
>
>
> Index: lily/skyline.cc
> diff --git a/lily/skyline.cc b/lily/skyline.cc
> index  
> b6ea6b791bbbfa09aaa28e91da0a619c33085890..8f62710b0947fa8e48ffcbc7e181cba4709c96de
>   
> 100644
> --- a/lily/skyline.cc
> +++ b/lily/skyline.cc
> @@ -514,6 +514,7 @@ Skyline::distance (Skyline const &other, Real  
> horizon_padding) const
>
>     Skyline const *padded_this = this;
>     Skyline const *padded_other = &other;
> +  bool created_tmp_skylines = false;
>
>     /*
>       For systems, padding is not added at creation time.  Padding is
> @@ -525,6 +526,7 @@ Skyline::distance (Skyline const &other, Real  
> horizon_padding) const
>       {
>         padded_this = new Skyline (*padded_this, horizon_padding, X_AXIS);
>         padded_other = new Skyline (*padded_other, horizon_padding, X_AXIS);
> +      created_tmp_skylines = true;
>       }
>
>     list<Building>::const_iterator i = padded_this->buildings_.begin ();
> @@ -544,6 +546,13 @@ Skyline::distance (Skyline const &other, Real  
> horizon_padding) const
>           j++;
>         start = end;
>       }
> +
> +  if (created_tmp_skylines)
> +    {
> +      delete padded_this;
> +      delete padded_other;
> +    }
> +
>     return dist;
>   }

Skylines are smobs.  The usual way to delete them would be to unprotect
them once they have been registered by some garbage-collectable object
(or a SCM variable that is being used for accessing them).

-- 
David Kastrup