[[PATCH setup topic/libsolv] 1/2] packagedb::removeEmptyCategories: Don't remove "Base"

[Jon Turney]

On 28/10/2017 13:29, Ken Brown wrote:
> This can be empty if no setup.ini files are found.  Removing it causes
> setup to hang.
> ---
>   package_db.cc | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package_db.cc b/package_db.cc
> index ac9387c..b104073 100644
> --- a/package_db.cc
> +++ b/package_db.cc
> @@ -596,7 +596,7 @@ packagedb::removeEmptyCategories()
>   {
>     for (packagedb::categoriesType::iterator n = packagedb::categories.begin();
>          n != packagedb::categories.end(); ++n)
> -    if (!n->second.size())
> +    if (!n->second.size() && n->first != "Base")
>         {
>           Log (LOG_BABBLE) << "Removing empty category " << n->first << endLog;
>           packagedb::categories.erase (n++);
> 

Hmm... now I remember my other concerns about this piece of code: as 
written, it's just wrong.

1. Applying erase to packagedb:categories invalidates the iterator
2. We're incrementing the iterator after doing an erase, so even if the 
iterator was still valid, we skip checking if the following category is 
empty

So maybe the right way to fix this is as attached:

I need to stare as this a bit more to understand where the 'base' 
category is coming from when we have no setup.ini...
-------------- next part --------------
From db1028c2ec19fd84366663b1384888dd33241202 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Sun, 29 Oct 2017 17:20:51 +0000
Subject: [PATCH setup] Fix invalid iterator use in
 packagedb::removeEmptyCategories()

---
 package_db.cc | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/package_db.cc b/package_db.cc
index 4d2ef4d..9de0875 100644
--- a/package_db.cc
+++ b/package_db.cc
@@ -593,13 +593,22 @@ packagedb::defaultTrust (SolverTasks &q, SolverSolution::updateMode mode, bool t
 void
 packagedb::removeEmptyCategories()
 {
+  std::vector<std::string> empty;
+
   for (packagedb::categoriesType::iterator n = packagedb::categories.begin();
        n != packagedb::categories.end(); ++n)
     if (!n->second.size())
       {
-        Log (LOG_BABBLE) << "Removing empty category " << n->first << endLog;
-        packagedb::categories.erase (n++);
+        empty.push_back(n->first);
       }
+
+  for (unsigned int i = 0; i < empty.size(); ++i)
+    {
+      packagedb::categoriesType::iterator n = packagedb::categories.find(empty[i]);
+      Log (LOG_BABBLE) << "Removing empty category " << empty[i] << endLog;
+      if (n != packagedb::categories.end())
+        packagedb::categories.erase(n);
+    }
 }
 
 void
-- 
2.14.3