Permalink
Browse files

Fixed: memory leak in Settings dialog

  • Loading branch information...
1 parent bf4cc7f commit e7eee533f26b9ef9c13ecc1dc9536ffb4f209d23 @Mezomish committed Apr 25, 2012
Showing with 3 additions and 1 deletion.
  1. +3 −1 src/app/ui/settings/MultiPage.cpp
@@ -143,7 +143,9 @@ void MultiPage::changeCurrentItem(QTreeWidgetItem* it, QTreeWidgetItem*) {
w->hide();
}
while ( mpInt_->panelLayout_->count() > 0 ) {
- mpInt_->panelLayout_->removeItem(mpInt_->panelLayout_->itemAt(0));
+ QLayoutItem* item = mpInt_->panelLayout_->itemAt(0);
+ mpInt_->panelLayout_->removeItem(item);
+ delete item;
}
mpInt_->panelLayout_->addWidget(page);
page->show();

2 comments on commit e7eee53

@spumer
spumer commented on e7eee53 May 4, 2012

Ммм... может быть здесь лучше сделать так?:

QLayoutItem* item;
while ( mpInt_->panelLayout_->count() > 0 ) {
item = mpInt_->panelLayout_->itemAt(0);
mpInt_->panelLayout_->removeItem(item);
delete item;
}

Или хотя бы так:

for (QLayoutItem* item; mpInt_->panelLayout_->count() > 0; delete item ) {
item = mpInt_->panelLayout_->itemAt(0);
mpInt_->panelLayout_->removeItem(item);
}

@Mezomish
Owner

В теории и по правилам - да, но на практике это то, что называется premature optimization, потому что:

  1. создание переменной-указателя на стеке - очень дешёвая операция
  2. while там на самом деле для подстраховки, а в реальных условиях в том лэйауте всегда находится не более одного виджета, так что цикл будет выполнен не более одного раза. В случае отсутствия элементов не будет выполнен ни разу, так что ненужная переменная вообще не будет создана (в отличие от предложенных вариантов).
Please sign in to comment.