[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fix memleak: temporary skyline objects for systems were never delete
From: |
David Kastrup |
Subject: |
Re: Fix memleak: temporary skyline objects for systems were never deleted (issue 4923048) |
Date: |
Wed, 24 Aug 2011 08:38:14 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) |
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