Record all available sources for descriptions

Avoiding duplications is one thing, but it could also be handy to know
all sources for a description so that you can e.g. avoid opening another
file or similar such if its also present in one you already have open.

Given how the structures were set up this was probably intended all
along and either worked at some point or was abandoned for the
duplicates that we started culling heavily with the introduction of
MultiArch as at that point they blew up the cache and needed to be dealt
with.

1 files changed, 55 insertions, 53 deletions

diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc
index de04d1313..dd79967c0 100644
--- a/apt-pkg/pkgcachegen.cc
+++ b/apt-pkg/pkgcachegen.cc
@@ -45,9 +45,6 @@ template<class T> using Dynamic = pkgCacheGenerator::Dynamic<T>;
 typedef std::vector<pkgIndexFile *>::iterator FileIterator;
 template <typename Iter> std::vector<Iter*> pkgCacheGenerator::Dynamic<Iter>::toReMap;
 
-static bool IsDuplicateDescription(pkgCache &Cache, pkgCache::DescIterator Desc,
-			    std::string_view CurMd5, std::string const &CurLang);
-
 using std::string;
 using std::string_view;
 
@@ -351,14 +348,10 @@ bool pkgCacheGenerator::MergeListPackage(ListParser &List, pkgCache::PkgIterator
 	 continue;
 
       map_stringitem_t md5idx = VerDesc->md5sum;
-      for (std::vector<std::string>::const_iterator CurLang = availDesc.begin(); CurLang != availDesc.end(); ++CurLang)
+      for (auto const &CurLang : availDesc)
       {
-	 // don't add a new description if we have one for the given
-	 // md5 && language
-	 if (IsDuplicateDescription(Cache, VerDesc, CurMd5, *CurLang) == true)
-	    continue;
-
-	 AddNewDescription(List, Ver, *CurLang, CurMd5, md5idx);
+	 if (not AddNewDescription(List, Ver, CurLang, CurMd5, md5idx))
+	    return false;
       }
 
       // we can stop here as all "same" versions will share the description
@@ -524,39 +517,55 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
       }
    }
 
-   // We haven't found reusable descriptions, so add the first description(s)
    map_stringitem_t md5idx = Ver->DescriptionList == 0 ? 0 : Ver.DescriptionList()->md5sum;
    for (auto const &CurLang : List.AvailableDescriptionLanguages())
-   {
-      if (Ver->DescriptionList != 0 && IsDuplicateDescription(Cache, Ver.DescriptionList(), CurMd5, CurLang))
-	 continue;
       if (not AddNewDescription(List, Ver, CurLang, CurMd5, md5idx))
 	 return false;
-   }
    return true;
 }
 									/*}}}*/
-bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterator &Ver, std::string const &lang, std::string_view CurMd5, map_stringitem_t &md5idx) /*{{{*/
+// findDescription							/*{{{*/
+static bool findDescription(pkgCache &Cache, pkgCache::DescIterator &Desc,
+			    std::string_view CurMd5, std::string_view const CurLang)
+{
+   // Descriptions in the same link-list have all the same md5
+   if (Desc.end() || Cache.ViewString(Desc->md5sum) != CurMd5)
+      return false;
+   for (; not Desc.end(); ++Desc)
+      if (CurLang == Cache.ViewString(Desc->language_code))
+	 return true;
+   return false;
+}
+									/*}}}*/
+bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterator &Ver, std::string const &CurLang, std::string_view CurMd5, map_stringitem_t &md5idx) /*{{{*/
 {
-   pkgCache::DescIterator Desc;
+   pkgCache::DescIterator Desc = Ver.DescriptionList();
    Dynamic<pkgCache::DescIterator> DynDesc(Desc);
 
-   map_pointer<pkgCache::Description> const descindex = NewDescription(Desc, lang, CurMd5, md5idx);
-   if (unlikely(descindex == 0))
-      return _error->Error(_("Error occurred while processing %s (%s%d)"),
-	    Ver.ParentPkg().Name(), "NewDescription", 1);
+   // don't add a new description if we have one for the given md5 && language
+   if (not findDescription(Cache, Desc, CurMd5, CurLang))
+   {
+      map_pointer<pkgCache::Description> const descindex = NewDescription(Desc, CurLang, CurMd5, md5idx);
+      if (unlikely(descindex == 0))
+	 return _error->Error(_("Error occurred while processing %s (%s%d)"),
+	       Ver.ParentPkg().Name(), "NewDescription", 1);
 
-   md5idx = Desc->md5sum;
-   Desc->ParentPkg = Ver.ParentPkg().MapPointer();
+      md5idx = Desc->md5sum;
+      Desc->ParentPkg = Ver.ParentPkg().MapPointer();
 
-   // we add at the end, so that the start is constant as we need
-   // that to be able to efficiently share these lists
-   pkgCache::DescIterator VerDesc = Ver.DescriptionList(); // old value might be invalid after ReMap
-   for (;VerDesc.end() == false && VerDesc->NextDesc != 0; ++VerDesc);
-   map_pointer<pkgCache::Description> * const LastNextDesc = (VerDesc.end() == true) ? &Ver->DescriptionList : &VerDesc->NextDesc;
-   *LastNextDesc = descindex;
+      // we add at the end, so that the start is constant as we need
+      // that to be able to efficiently share these lists
+      if (Ver->DescriptionList == 0)
+	 Ver->DescriptionList = descindex;
+      else
+      {
+	 auto VerDesc = Ver.DescriptionList();
+	 for (; VerDesc->NextDesc != 0; ++VerDesc);
+	 VerDesc->NextDesc = descindex;
+      }
+   }
 
-   if (NewFileDesc(Desc,List) == false)
+   if (not NewFileDesc(Desc, List))
       return _error->Error(_("Error occurred while processing %s (%s%d)"),
 	    Ver.ParentPkg().Name(), "NewFileDesc", 1);
 
@@ -932,31 +941,37 @@ map_pointer<pkgCache::Version> pkgCacheGenerator::NewVersion(pkgCache::VerIterat
 }
 									/*}}}*/
 // CacheGenerator::NewFileDesc - Create a new File<->Desc association	/*{{{*/
-// ---------------------------------------------------------------------
-/* */
 bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc,
 				   ListParser &List)
 {
    if (CurrentFile == nullptr)
       return true;
 
+   auto const DFFile = map_pointer<pkgCache::PackageFile>{NarrowOffset(CurrentFile - Cache.PkgFileP)};
+   auto const DFOffset = List.Offset();
+   for (auto DF = Desc.FileList(); not DF.end(); ++DF)
+      if (DF->File == DFFile && DF->Offset == DFOffset)
+	 return true;
+
    // Get a structure
    auto const DescFile = AllocateInMap<pkgCache::DescFile>();
    if (DescFile == 0)
       return false;
 
    pkgCache::DescFileIterator DF(Cache,Cache.DescFileP + DescFile);
-   DF->File = map_pointer<pkgCache::PackageFile>{NarrowOffset(CurrentFile - Cache.PkgFileP)};
+   DF->File = DFFile;
+   DF->Offset = DFOffset;
 
    // Link it to the end of the list
-   map_pointer<pkgCache::DescFile> *Last = &Desc->FileList;
-   for (pkgCache::DescFileIterator D = Desc.FileList(); D.end() == false; ++D)
-      Last = &D->NextFile;
-
-   DF->NextFile = *Last;
-   *Last = DF.MapPointer();
+   if (Desc->FileList == 0)
+      Desc->FileList = DescFile;
+   else
+   {
+      auto Last = Desc.FileList();
+      for (; Last->NextFile != 0; ++Last);
+      Last->NextFile = DescFile;
+   }
 
-   DF->Offset = List.Offset();
    auto const Size = List.Size();
    if (Cache.HeaderP->MaxDescFileSize < Size)
       Cache.HeaderP->MaxDescFileSize = Size;
@@ -1886,19 +1901,6 @@ bool pkgCacheGenerator::MakeOnlyStatusCache(OpProgress *Progress,DynamicMMap **O
    return true;
 }
 									/*}}}*/
-// IsDuplicateDescription						/*{{{*/
-static bool IsDuplicateDescription(pkgCache &Cache, pkgCache::DescIterator Desc,
-			    std::string_view CurMd5, std::string const &CurLang)
-{
-   // Descriptions in the same link-list have all the same md5
-   if (Desc.end() == true || Cache.ViewString(Desc->md5sum) != CurMd5)
-      return false;
-   for (; Desc.end() == false; ++Desc)
-      if (Desc.LanguageCode() == CurLang)
-	 return true;
-   return false;
-}
-									/*}}}*/
 
 pkgCacheListParser::pkgCacheListParser() : Owner(NULL), OldDepLast(NULL), d(NULL) {}
 pkgCacheListParser::~pkgCacheListParser() {}