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

[Ken Brown <kbrown at cornell dot edu>]

On 10/29/2017 1:24 PM, Jon Turney wrote:
> 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:

Yes.  I wrongly jumped to the conclusion that removing Base was the issue.

> I need to stare as this a bit more to understand where the 'base' 
> category is coming from when we have no setup.ini...

I guess it's created implicitly in the 'for' statement in 
packagedb::makeBase().  That's probably a mistake.

Ken