Merge branch 'fix/stats' into 'main'

`stats` rabbithole: Do not overflow ver/desc<->file relation counters

See merge request apt-team/apt!441

4 files changed, 111 insertions, 100 deletions

diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc
index 16df1704e..bccd7798f 100644
--- a/apt-pkg/pkgcache.cc
+++ b/apt-pkg/pkgcache.cc
@@ -67,7 +67,7 @@ pkgCache::Header::Header()
    APT_HEADER_SET(PackageSz, sizeof(pkgCache::Package));
    APT_HEADER_SET(ReleaseFileSz, sizeof(pkgCache::ReleaseFile));
    APT_HEADER_SET(PackageFileSz, sizeof(pkgCache::PackageFile));
-   APT_HEADER_SET(VersionSz, sizeof(pkgCache::Version));
+   APT_HEADER_SET(VersionSz, sizeof(pkgCache::Version) + sizeof(pkgCache::Version::Extra));
    APT_HEADER_SET(DescriptionSz, sizeof(pkgCache::Description));
    APT_HEADER_SET(DependencySz, sizeof(pkgCache::Dependency));
    APT_HEADER_SET(DependencyDataSz, sizeof(pkgCache::DependencyData));
diff --git a/apt-pkg/pkgcache.h b/apt-pkg/pkgcache.h
index 476a01514..0b61b0048 100644
--- a/apt-pkg/pkgcache.h
+++ b/apt-pkg/pkgcache.h
@@ -341,8 +341,13 @@ struct pkgCache::Header
    map_id_t DependsDataCount;
    map_fileid_t ReleaseFileCount;
    map_fileid_t PackageFileCount;
+#if APT_PKG_ABI <= 600
    map_fileid_t VerFileCount;
    map_fileid_t DescFileCount;
+#else
+   map_id_t VerFileCount;
+   map_id_t DescFileCount;
+#endif
    map_id_t ProvidesCount;
 
    /** \brief index of the first PackageFile structure
@@ -599,8 +604,9 @@ struct pkgCache::VerFile
    map_pointer<VerFile> NextFile;
    /** \brief position in the package file */
    map_filesize_t Offset;         // File offset
-   /** @TODO document pkgCache::VerFile::Size */
-   map_filesize_t Size;
+#if APT_PKG_ABI <= 600
+   [[deprecated("No usage in src:apt, try MaxVerFileSize instead")]] map_filesize_t Size;
+#endif
 };
 									/*}}}*/
 // DescFile structure							/*{{{*/
@@ -613,8 +619,9 @@ struct pkgCache::DescFile
    map_pointer<DescFile> NextFile;
    /** \brief position in the file */
    map_filesize_t Offset;         // File offset
-   /** @TODO document pkgCache::DescFile::Size */
-   map_filesize_t Size;
+#if APT_PKG_ABI <= 600
+   [[deprecated("No usage in src:apt, try MaxDescFileSize instead")]] map_filesize_t Size;
+#endif
 };
 									/*}}}*/
 // Version structure							/*{{{*/
diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc
index aaa969a8d..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
@@ -513,47 +506,66 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
       a version with a description of the same MD5 - if so we reuse this
       description group instead of creating our own for this version */
    for (pkgCache::PkgIterator P = Grp.PackageList();
-	P.end() == false; P = Grp.NextPkg(P))
+	not P.end() && Ver->DescriptionList == 0; P = Grp.NextPkg(P))
    {
-      for (pkgCache::VerIterator V = P.VersionList();
-	   V.end() == false; ++V)
+      for (pkgCache::VerIterator V = P.VersionList(); not V.end(); ++V)
       {
 	 if (V->DescriptionList == 0 || Cache.ViewString(V.DescriptionList()->md5sum) != CurMd5)
 	    continue;
 	 Ver->DescriptionList = V->DescriptionList;
+	 break;
       }
    }
 
-   // We haven't found reusable descriptions, so add the first description(s)
    map_stringitem_t md5idx = Ver->DescriptionList == 0 ? 0 : Ver.DescriptionList()->md5sum;
-   std::vector<std::string> availDesc = List.AvailableDescriptionLanguages();
-   for (std::vector<std::string>::const_iterator CurLang = availDesc.begin(); CurLang != availDesc.end(); ++CurLang)
-      if (AddNewDescription(List, Ver, *CurLang, CurMd5, md5idx) == false)
+   for (auto const &CurLang : List.AvailableDescriptionLanguages())
+      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)
 {
-   pkgCache::DescIterator Desc;
+   // 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 = 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);
 
@@ -854,9 +866,14 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver,
    *Last = VF.MapPointer();
 
    VF->Offset = List.Offset();
-   VF->Size = List.Size();
-   if (Cache.HeaderP->MaxVerFileSize < VF->Size)
-      Cache.HeaderP->MaxVerFileSize = VF->Size;
+   auto const Size = List.Size();
+   if (Cache.HeaderP->MaxVerFileSize < Size)
+      Cache.HeaderP->MaxVerFileSize = Size;
+#if APT_PKG_ABI <= 600
+   APT_IGNORE_DEPRECATED_PUSH
+   VF->Size = Size;
+   APT_IGNORE_DEPRECATED_POP
+#endif
    Cache.HeaderP->VerFileCount++;
 
    return true;
@@ -924,34 +941,45 @@ 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();
-   DF->Size = List.Size();
-   if (Cache.HeaderP->MaxDescFileSize < DF->Size)
-      Cache.HeaderP->MaxDescFileSize = DF->Size;
+   auto const Size = List.Size();
+   if (Cache.HeaderP->MaxDescFileSize < Size)
+      Cache.HeaderP->MaxDescFileSize = Size;
+#if APT_PKG_ABI <= 600
+   APT_IGNORE_DEPRECATED_PUSH
+   DF->Size = Size;
+   APT_IGNORE_DEPRECATED_POP
+#endif
    Cache.HeaderP->DescFileCount++;
 
    return true;
@@ -1873,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() {}
diff --git a/cmdline/apt-cache.cc b/cmdline/apt-cache.cc
index 7fdd4dff2..67dde67a6 100644
--- a/cmdline/apt-cache.cc
+++ b/cmdline/apt-cache.cc
@@ -87,9 +87,10 @@ static bool DumpPackage(CommandLine &CmdL)
 	 cout << endl;
 	 for (pkgCache::DescIterator D = Cur.DescriptionList(); D.end() == false; ++D)
 	 {
-	    cout << " Description Language: " << D.LanguageCode() << endl
-		 << "                 File: " << D.FileList().File().FileName() << endl
-		 << "                  MD5: " << D.md5() << endl;
+	    cout << " Description Language: " << D.LanguageCode() << '\n';
+	    for (auto DF = D.FileList(); not DF.end(); ++DF)
+	       cout << "                 File: " << DF.File().FileName() << '\n';
+	    cout << "                  MD5: " << D.md5() << '\n';
 	 }
 	 cout << endl;
       }
@@ -464,7 +465,6 @@ static bool DumpAvail(CommandLine &)
    stdoutfd.OpenDescriptor(STDOUT_FILENO, FileFd::WriteOnly, false);
 
    // Iterate over all the package files and write them out.
-   char *Buffer = new char[Cache->HeaderP->MaxVerFileSize+10];
    for (pkgCache::VerFile **J = VFList; *J != 0;)
    {
       pkgCache::PkgFileIterator File(*Cache, Cache->PkgFileP + (*J)->File);
@@ -472,40 +472,23 @@ static bool DumpAvail(CommandLine &)
       FileFd PkgF(File.FileName(),FileFd::ReadOnly, FileFd::Extension);
       if (_error->PendingError() == true)
 	 break;
-      
-      /* Write all of the records from this package file, since we
-       	 already did locality sorting we can now just seek through the
-       	 file in read order. We apply 1 more optimization here, since often
-       	 there will be < 1 byte gaps between records (for the \n) we read that
-       	 into the next buffer and offset a bit.. */
-      unsigned long Pos = 0;
+
+      pkgTagFile tagsfile(&PkgF, Cache->HeaderP->MaxVerFileSize);
+      pkgTagSection Tags;
+
       for (; *J != 0; J++)
       {
 	 if (Cache->PkgFileP + (*J)->File != File)
 	    break;
-	 
-	 const pkgCache::VerFile &VF = **J;
 
-	 // Read the record and then write it out again.
-	 unsigned long Jitter = VF.Offset - Pos;
-	 if (Jitter > 8)
-	 {
-	    if (PkgF.Seek(VF.Offset) == false)
-	       break;
-	    Jitter = 0;
-	 }
-	 
-	 if (PkgF.Read(Buffer,VF.Size + Jitter) == false)
+	 const pkgCache::VerFile &VF = **J;
+	 if (not tagsfile.Jump(Tags, VF.Offset))
 	    break;
-	 Buffer[VF.Size + Jitter] = '\n';
 
-	 // See above..
 	 if ((File->Flags & pkgCache::Flag::NotSource) == pkgCache::Flag::NotSource)
 	 {
-	    pkgTagSection Tags;
-	    if (Tags.Scan(Buffer+Jitter,VF.Size+1) == false ||
-		Tags.Write(stdoutfd, NULL, RW) == false ||
-		stdoutfd.Write("\n", 1) == false)
+	    if (not Tags.Write(stdoutfd, NULL, RW) ||
+		  not stdoutfd.Write("\n", 1))
 	    {
 	       _error->Error("Internal Error, Unable to parse a package record");
 	       break;
@@ -513,18 +496,24 @@ static bool DumpAvail(CommandLine &)
 	 }
 	 else
 	 {
-	    if (stdoutfd.Write(Buffer + Jitter, VF.Size + 1) == false)
+	    char const *Start, *Stop;
+	    Tags.GetSection(Start, Stop);
+	    for (; Start < Stop; --Stop)
+	       if (std::string_view{"\n\r"}.find(*(Stop - 1)) == std::string_view::npos)
+		  break;
+	    if (not stdoutfd.Write(Start, Stop - Start) ||
+		not stdoutfd.Write("\n\n", 2))
+	    {
+	       _error->Error("Internal Error, Unable to parse a package record");
 	       break;
+	    }
 	 }
-
-	 Pos = VF.Offset + VF.Size;
       }
 
-      if (_error->PendingError() == true)
+      if (_error->PendingError())
          break;
    }
 
-   delete [] Buffer;
    delete [] VFList;
    return !_error->PendingError();
 }