[RFC][PATCH] Fix for potential memory leak in GuiLyXFiles::updateContents

Enrico Forestieri forenr at lyx.org
Thu Feb 20 12:22:52 UTC 2020


On Thu, Feb 20, 2020 at 09:37:12AM +0100, Stephan Witt wrote:
> Hi all,
> 
> the last one of the potential memory leaks is a little bit trickier.
> 
> The loop logic for subcatItem if cats contains catsave doesn’t guarantee
> a match so a check for the match is needed after all.
> 
> This makes me unsure what should be done in this case.
> Ignore it or warn about it or try something else?

Please, see suggestion below.

> 
> Stephan
>  

> [-- mutt.octet.filter file type: "diff output text" --]
> 
> diff --git a/src/frontends/qt/GuiLyXFiles.cpp b/src/frontends/qt/GuiLyXFiles.cpp
> index 096a390d64..d6f283c397 100644
> --- a/src/frontends/qt/GuiLyXFiles.cpp
> +++ b/src/frontends/qt/GuiLyXFiles.cpp
> @@ -445,7 +445,7 @@ void GuiLyXFiles::updateContents()
>  		if (subcat.isEmpty())
>  			catItem->addChild(item);
>  		else {
> -			QTreeWidgetItem * subcatItem = new QTreeWidgetItem();
> +			QTreeWidgetItem * subcatItem = 0;

These days nullptr is preferred to an unadorned 0.

>  			if (cats.contains(catsave)) {
>  				QList<QTreeWidgetItem *> pcats = filesLW->findItems(cat, Qt::MatchExactly);
>  				for (int iit = 0; iit < pcats.size(); ++iit) {
> @@ -457,12 +457,15 @@ void GuiLyXFiles::updateContents()
>  					}
>  				}

Change this line
>  			} else {

with
  			}
			if (!subcatItem) {
> +				subcatItem = new QTreeWidgetItem();
>  				subcatItem->setText(0, subcat);
>  				subcatItem->setIcon(0, file_icon);
>  				cats << catsave;
>  			}

Drop the following change

> -			subcatItem->addChild(item);
> -			catItem->addChild(subcatItem);
> +			if (subcatItem) {
> +				subcatItem->addChild(item);
> +				catItem->addChild(subcatItem);
> +			}
>  		}
>  		++it;
>  	}


-- 
Enrico


More information about the lyx-devel mailing list