On Wed, Aug 24, 2011 at 3:38 AM, David Kastrup <d...@gnu.org> wrote:
> reinhold.kainho...@gmail.com 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).

They are simple smobs, though, so this pattern here is not uncommon.
You could also try to allocate them on the stack.

--
Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen

_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Reply via email to