From c3587c0d9de852eca11d9bbc004095d54115eda4 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 24 Feb 2020 17:08:34 +0100 Subject: Replace map_pointer_t with map_pointer This is a first step to a type safe cache, adding typing information everywhere. Next, we'll replace map_pointer implementation with a type safe one. --- apt-pkg/cacheiterators.h | 12 ++--- apt-pkg/pkgcache.h | 96 ++++++++++++++++++++-------------------- apt-pkg/pkgcachegen.cc | 72 +++++++++++++++--------------- apt-pkg/pkgcachegen.h | 16 +++---- apt-private/private-cachefile.cc | 4 +- apt-private/private-cachefile.h | 10 ++--- cmdline/apt-cache.cc | 8 ++-- 7 files changed, 109 insertions(+), 109 deletions(-) diff --git a/apt-pkg/cacheiterators.h b/apt-pkg/cacheiterators.h index ff2b65cdf..7f853558b 100644 --- a/apt-pkg/cacheiterators.h +++ b/apt-pkg/cacheiterators.h @@ -321,15 +321,15 @@ class pkgCache::DepIterator : public Iterator { struct DependencyProxy { map_stringitem_t &Version; - map_pointer_t &Package; + map_pointer &Package; map_id_t &ID; unsigned char &Type; unsigned char &CompareOp; - map_pointer_t &ParentVer; - map_pointer_t &DependencyData; - map_pointer_t &NextRevDepends; - map_pointer_t &NextDepends; - map_pointer_t &NextData; + map_pointer &ParentVer; + map_pointer &DependencyData; + map_pointer &NextRevDepends; + map_pointer &NextDepends; + map_pointer &NextData; DependencyProxy const * operator->() const { return this; } DependencyProxy * operator->() { return this; } }; diff --git a/apt-pkg/pkgcache.h b/apt-pkg/pkgcache.h index 84fc56db8..e5a1a81eb 100644 --- a/apt-pkg/pkgcache.h +++ b/apt-pkg/pkgcache.h @@ -93,9 +93,9 @@ typedef uint32_t map_id_t; // some files get an id, too, but in far less absolute numbers typedef uint16_t map_fileid_t; // relative pointer from cache start -typedef uint32_t map_pointer_t; +template using map_pointer = uint32_t; // same as the previous, but documented to be to a string item -typedef map_pointer_t map_stringitem_t; +typedef map_pointer map_stringitem_t; // we have only a small amount of flags for each item typedef uint8_t map_flags_t; typedef uint8_t map_number_t; @@ -325,16 +325,16 @@ struct pkgCache::Header The PackageFile structures are singly linked lists that represent all package files that have been merged into the cache. */ - map_pointer_t FileList; + map_pointer FileList; /** \brief index of the first ReleaseFile structure */ - map_pointer_t RlsFileList; + map_pointer RlsFileList; /** \brief String representing the version system used */ - map_pointer_t VerSysName; + map_stringitem_t VerSysName; /** \brief native architecture the cache was built against */ - map_pointer_t Architecture; + map_stringitem_t Architecture; /** \brief all architectures the cache was built against */ - map_pointer_t Architectures; + map_stringitem_t Architectures; /** \brief The maximum size of a raw entry from the original Package file */ map_filesize_t MaxVerFileSize; /** \brief The maximum size of a raw entry from the original Translation file */ @@ -363,10 +363,10 @@ struct pkgCache::Header uint32_t HashTableSize; uint32_t GetHashTableSize() const { return HashTableSize; } void SetHashTableSize(unsigned int const sz) { HashTableSize = sz; } - map_pointer_t GetArchitectures() const { return Architectures; } - void SetArchitectures(map_pointer_t const idx) { Architectures = idx; } - map_pointer_t * PkgHashTableP() const { return (map_pointer_t*) (this + 1); } - map_pointer_t * GrpHashTableP() const { return PkgHashTableP() + GetHashTableSize(); } + map_stringitem_t GetArchitectures() const { return Architectures; } + void SetArchitectures(map_stringitem_t const idx) { Architectures = idx; } + map_pointer * PkgHashTableP() const { return (map_pointer*) (this + 1); } + map_pointer * GrpHashTableP() const { return reinterpret_cast *>(PkgHashTableP() + GetHashTableSize()); } /** \brief Hash of the file (TODO: Rename) */ map_filesize_small_t CacheFileSize; @@ -393,17 +393,17 @@ struct pkgCache::Group // Linked List /** \brief Link to the first package which belongs to the group */ - map_pointer_t FirstPackage; // Package + map_pointer FirstPackage; /** \brief Link to the last package which belongs to the group */ - map_pointer_t LastPackage; // Package + map_pointer LastPackage; /** \brief Link to the next Group */ - map_pointer_t Next; // Group + map_pointer Next; /** \brief unique sequel ID */ map_id_t ID; /** \brief List of binary produces by source package with this name. */ - map_pointer_t VersionsInSource; // Version + map_pointer VersionsInSource; }; /*}}}*/ @@ -432,19 +432,19 @@ struct pkgCache::Package versions of a package can be cleanly handled by the system. Furthermore, this linked list is guaranteed to be sorted from Highest version to lowest version with no duplicate entries. */ - map_pointer_t VersionList; // Version + map_pointer VersionList; /** \brief index to the installed version */ - map_pointer_t CurrentVer; // Version + map_pointer CurrentVer; /** \brief index of the group this package belongs to */ - map_pointer_t Group; // Group the Package belongs to + map_pointer Group; // Linked list /** \brief Link to the next package in the same bucket */ - map_pointer_t NextPackage; // Package + map_pointer NextPackage; /** \brief List of all dependencies on this package */ - map_pointer_t RevDepends; // Dependency + map_pointer RevDepends; /** \brief List of all "packages" this package provide */ - map_pointer_t ProvidesList; // Provides + map_pointer ProvidesList; // Install/Remove/Purge etc /** \brief state that the user wishes the package to be in */ @@ -504,7 +504,7 @@ struct pkgCache::ReleaseFile // Linked list /** \brief Link to the next ReleaseFile in the Cache */ - map_pointer_t NextFile; + map_pointer NextFile; /** \brief unique sequel ID */ map_fileid_t ID; }; @@ -520,7 +520,7 @@ struct pkgCache::PackageFile /** \brief physical disk file that this PackageFile represents */ map_stringitem_t FileName; /** \brief the release information */ - map_pointer_t Release; + map_pointer Release; map_stringitem_t Component; map_stringitem_t Architecture; @@ -543,7 +543,7 @@ struct pkgCache::PackageFile // Linked list /** \brief Link to the next PackageFile in the Cache */ - map_pointer_t NextFile; // PackageFile + map_pointer NextFile; /** \brief unique sequel ID */ map_fileid_t ID; }; @@ -556,9 +556,9 @@ struct pkgCache::PackageFile struct pkgCache::VerFile { /** \brief index of the package file that this version was found in */ - map_pointer_t File; // PackageFile + map_pointer File; /** \brief next step in the linked list */ - map_pointer_t NextFile; // PkgVerFile + map_pointer NextFile; /** \brief position in the package file */ map_filesize_t Offset; // File offset /** @TODO document pkgCache::VerFile::Size */ @@ -570,9 +570,9 @@ struct pkgCache::VerFile struct pkgCache::DescFile { /** \brief index of the file that this description was found in */ - map_pointer_t File; // PackageFile + map_pointer File; /** \brief next step in the linked list */ - map_pointer_t NextFile; // PkgVerFile + map_pointer NextFile; /** \brief position in the file */ map_filesize_t Offset; // File offset /** @TODO document pkgCache::DescFile::Size */ @@ -619,19 +619,19 @@ struct pkgCache::Version applies to. If FileList is 0 then this is a blank version. The structure should also have a 0 in all other fields excluding pkgCache::Version::VerStr and Possibly pkgCache::Version::NextVer. */ - map_pointer_t FileList; // VerFile + map_pointer FileList; /** \brief next (lower or equal) version in the linked list */ - map_pointer_t NextVer; // Version + map_pointer NextVer; /** \brief next description in the linked list */ - map_pointer_t DescriptionList; // Description + map_pointer DescriptionList; /** \brief base of the dependency list */ - map_pointer_t DependsList; // Dependency + map_pointer DependsList; /** \brief links to the owning package This allows reverse dependencies to determine the package */ - map_pointer_t ParentPkg; // Package + map_pointer ParentPkg; /** \brief list of pkgCache::Provides */ - map_pointer_t ProvidesList; // Provides + map_pointer ProvidesList; /** \brief archive size for this version @@ -649,7 +649,7 @@ struct pkgCache::Version /** \brief parsed priority value */ map_number_t Priority; /** \brief next version in the source package (might be different binary) */ - map_pointer_t NextInSource; // Version + map_pointer NextInSource; }; /*}}}*/ // Description structure /*{{{*/ @@ -668,11 +668,11 @@ struct pkgCache::Description map_stringitem_t md5sum; /** @TODO document pkgCache::Description::FileList */ - map_pointer_t FileList; // DescFile + map_pointer FileList; /** \brief next translation for this description */ - map_pointer_t NextDesc; // Description + map_pointer NextDesc; /** \brief the text is a description of this package */ - map_pointer_t ParentPkg; // Package + map_pointer ParentPkg; /** \brief unique sequel ID */ map_id_t ID; @@ -693,7 +693,7 @@ struct pkgCache::DependencyData The generator will - if the package does not already exist - create a blank (no version records) package. */ - map_pointer_t Package; // Package + map_pointer Package; /** \brief Dependency type - Depends, Recommends, Conflicts, etc */ map_number_t Type; @@ -702,17 +702,17 @@ struct pkgCache::DependencyData If the high bit is set then it is a logical OR with the previous record. */ map_flags_t CompareOp; - map_pointer_t NextData; + map_pointer NextData; }; struct pkgCache::Dependency { - map_pointer_t DependencyData; // DependencyData + map_pointer DependencyData; /** \brief version of the package which has the depends */ - map_pointer_t ParentVer; // Version + map_pointer ParentVer; /** \brief next reverse dependency of this package */ - map_pointer_t NextRevDepends; // Dependency + map_pointer NextRevDepends; /** \brief next dependency of this version */ - map_pointer_t NextDepends; // Dependency + map_pointer NextDepends; /** \brief unique sequel ID */ map_id_t ID; @@ -730,9 +730,9 @@ struct pkgCache::Dependency struct pkgCache::Provides { /** \brief index of the package providing this */ - map_pointer_t ParentPkg; // Package + map_pointer ParentPkg; /** \brief index of the version this provide line applies to */ - map_pointer_t Version; // Version + map_pointer Version; /** \brief version in the provides line (if any) This version allows dependencies to depend on specific versions of a @@ -740,9 +740,9 @@ struct pkgCache::Provides map_stringitem_t ProvideVersion; map_flags_t Flags; /** \brief next provides (based of package) */ - map_pointer_t NextProvides; // Provides + map_pointer NextProvides; /** \brief next provides (based of version) */ - map_pointer_t NextPkgProv; // Provides + map_pointer NextPkgProv; }; /*}}}*/ diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 5a7272e84..7a6ada5a2 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -77,7 +77,7 @@ bool pkgCacheGenerator::Start() *Cache.HeaderP = pkgCache::Header(); // make room for the hashtables for packages and groups - if (Map.RawAllocate(2 * (Cache.HeaderP->GetHashTableSize() * sizeof(map_pointer_t))) == 0) + if (Map.RawAllocate(2 * (Cache.HeaderP->GetHashTableSize() * sizeof(map_pointer))) == 0) return false; map_stringitem_t const idxVerSysName = WriteStringInMap(_system->VS->Label); @@ -226,10 +226,10 @@ map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String) { return index; } /*}}}*/ -map_pointer_t pkgCacheGenerator::AllocateInMap(const unsigned long &size) {/*{{{*/ +map_pointer pkgCacheGenerator::AllocateInMap(const unsigned long &size) {/*{{{*/ size_t oldSize = Map.Size(); void const * const oldMap = Map.Data(); - map_pointer_t const index = Map.Allocate(size); + map_pointer const index = Map.Allocate(size); if (index != 0) ReMap(oldMap, Map.Data(), oldSize); return index; @@ -373,7 +373,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator { pkgCache::VerIterator Ver = Pkg.VersionList(); Dynamic DynVer(Ver); - map_pointer_t *LastVer = &Pkg->VersionList; + map_pointer *LastVer = &Pkg->VersionList; void const * oldMap = Map.Data(); auto Hash = List.VersionHash(); @@ -433,13 +433,13 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator } // Add a new version - map_pointer_t const verindex = NewVersion(Ver, Version, Pkg.Index(), Hash, *LastVer); + map_pointer const verindex = NewVersion(Ver, Version, Pkg.Index(), Hash, *LastVer); if (unlikely(verindex == 0)) return _error->Error(_("Error occurred while processing %s (%s%d)"), Pkg.Name(), "NewVersion", 1); if (oldMap != Map.Data()) - LastVer += static_cast(Map.Data()) - static_cast(oldMap); + LastVer += static_cast const *>(Map.Data()) - static_cast const *>(oldMap); *LastVer = verindex; if (unlikely(List.NewVersion(Ver) == false)) @@ -519,7 +519,7 @@ bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterato pkgCache::DescIterator Desc; Dynamic DynDesc(Desc); - map_pointer_t const descindex = NewDescription(Desc, lang, CurMd5, md5idx); + map_pointer 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); @@ -531,7 +531,7 @@ bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterato // 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_t * const LastNextDesc = (VerDesc.end() == true) ? &Ver->DescriptionList : &VerDesc->NextDesc; + map_pointer * const LastNextDesc = (VerDesc.end() == true) ? &Ver->DescriptionList : &VerDesc->NextDesc; *LastNextDesc = descindex; if (NewFileDesc(Desc,List) == false) @@ -553,7 +553,7 @@ bool pkgCacheGenerator::NewGroup(pkgCache::GrpIterator &Grp, StringView Name) return true; // Get a structure - map_pointer_t const Group = AllocateInMap(sizeof(pkgCache::Group)); + map_pointer const Group = AllocateInMap(sizeof(pkgCache::Group)); if (unlikely(Group == 0)) return false; @@ -566,7 +566,7 @@ bool pkgCacheGenerator::NewGroup(pkgCache::GrpIterator &Grp, StringView Name) // Insert it into the hash table unsigned long const Hash = Cache.Hash(Name); - map_pointer_t *insertAt = &Cache.HeaderP->GrpHashTableP()[Hash]; + map_pointer *insertAt = &Cache.HeaderP->GrpHashTableP()[Hash]; while (*insertAt != 0 && StringViewCompareFast(Name, Cache.ViewString((Cache.GrpP + *insertAt)->Name)) > 0) insertAt = &(Cache.GrpP + *insertAt)->Next; @@ -594,7 +594,7 @@ bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name, return true; // Get a structure - map_pointer_t const Package = AllocateInMap(sizeof(pkgCache::Package)); + map_pointer const Package = AllocateInMap(sizeof(pkgCache::Package)); if (unlikely(Package == 0)) return false; Pkg = pkgCache::PkgIterator(Cache,Cache.PkgP + Package); @@ -614,7 +614,7 @@ bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name, Grp->FirstPackage = Package; // Insert it into the hash table map_id_t const Hash = Cache.Hash(Name); - map_pointer_t *insertAt = &Cache.HeaderP->PkgHashTableP()[Hash]; + map_pointer *insertAt = &Cache.HeaderP->PkgHashTableP()[Hash]; while (*insertAt != 0 && StringViewCompareFast(Name, Cache.ViewString((Cache.GrpP + (Cache.PkgP + *insertAt)->Group)->Name)) > 0) insertAt = &(Cache.PkgP + *insertAt)->NextPackage; Pkg->NextPackage = *insertAt; @@ -678,7 +678,7 @@ bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name, continue; pkgCache::VerIterator Ver = Dep.ParentVer(); Dynamic DynVer(Ver); - map_pointer_t * unused = NULL; + map_pointer * unused = NULL; if (NewDepends(Pkg, Ver, Dep->Version, Dep->CompareOp, Dep->Type, unused) == false) return false; } @@ -747,7 +747,7 @@ bool pkgCacheGenerator::AddImplicitDepends(pkgCache::GrpIterator &G, { APT::StringView Arch = P.Arch() == NULL ? "" : P.Arch(); Dynamic DynArch(Arch); - map_pointer_t *OldDepLast = NULL; + map_pointer *OldDepLast = NULL; /* MultiArch handling introduces a lot of implicit Dependencies: - MultiArch: same → Co-Installable if they have the same version - All others conflict with all other group members */ @@ -787,7 +787,7 @@ bool pkgCacheGenerator::AddImplicitDepends(pkgCache::VerIterator &V, /* MultiArch handling introduces a lot of implicit Dependencies: - MultiArch: same → Co-Installable if they have the same version - All others conflict with all other group members */ - map_pointer_t *OldDepLast = NULL; + map_pointer *OldDepLast = NULL; bool const coInstall = ((V->MultiArch & pkgCache::Version::Same) == pkgCache::Version::Same); if (coInstall == true) { @@ -820,7 +820,7 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, return true; // Get a structure - map_pointer_t const VerFile = AllocateInMap(sizeof(pkgCache::VerFile)); + map_pointer const VerFile = AllocateInMap(sizeof(pkgCache::VerFile)); if (VerFile == 0) return false; @@ -828,7 +828,7 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, VF->File = CurrentFile - Cache.PkgFileP; // Link it to the end of the list - map_pointer_t *Last = &Ver->FileList; + map_pointer *Last = &Ver->FileList; for (pkgCache::VerFileIterator V = Ver.FileList(); V.end() == false; ++V) Last = &V->NextFile; VF->NextFile = *Last; @@ -846,14 +846,14 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, // CacheGenerator::NewVersion - Create a new Version /*{{{*/ // --------------------------------------------------------------------- /* This puts a version structure in the linked list */ -map_pointer_t pkgCacheGenerator::NewVersion(pkgCache::VerIterator &Ver, +map_pointer pkgCacheGenerator::NewVersion(pkgCache::VerIterator &Ver, APT::StringView const &VerStr, - map_pointer_t const ParentPkg, + map_pointer const ParentPkg, uint32_t Hash, - map_pointer_t const Next) + map_pointer const Next) { // Get a structure - map_pointer_t const Version = AllocateInMap(sizeof(pkgCache::Version)); + map_pointer const Version = AllocateInMap(sizeof(pkgCache::Version)); if (Version == 0) return 0; @@ -905,7 +905,7 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, return true; // Get a structure - map_pointer_t const DescFile = AllocateInMap(sizeof(pkgCache::DescFile)); + map_pointer const DescFile = AllocateInMap(sizeof(pkgCache::DescFile)); if (DescFile == 0) return false; @@ -913,7 +913,7 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, DF->File = CurrentFile - Cache.PkgFileP; // Link it to the end of the list - map_pointer_t *Last = &Desc->FileList; + map_pointer *Last = &Desc->FileList; for (pkgCache::DescFileIterator D = Desc.FileList(); D.end() == false; ++D) Last = &D->NextFile; @@ -932,13 +932,13 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, // CacheGenerator::NewDescription - Create a new Description /*{{{*/ // --------------------------------------------------------------------- /* This puts a description structure in the linked list */ -map_pointer_t pkgCacheGenerator::NewDescription(pkgCache::DescIterator &Desc, +map_pointer pkgCacheGenerator::NewDescription(pkgCache::DescIterator &Desc, const string &Lang, APT::StringView md5sum, map_stringitem_t const idxmd5str) { // Get a structure - map_pointer_t const Description = AllocateInMap(sizeof(pkgCache::Description)); + map_pointer const Description = AllocateInMap(sizeof(pkgCache::Description)); if (Description == 0) return 0; @@ -969,20 +969,20 @@ map_pointer_t pkgCacheGenerator::NewDescription(pkgCache::DescIterator &Desc, version and to the package that it is pointing to. */ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, pkgCache::VerIterator &Ver, - map_pointer_t const Version, + map_pointer const Version, uint8_t const Op, uint8_t const Type, - map_pointer_t* &OldDepLast) + map_pointer * &OldDepLast) { void const * const oldMap = Map.Data(); // Get a structure - map_pointer_t const Dependency = AllocateInMap(sizeof(pkgCache::Dependency)); + map_pointer const Dependency = AllocateInMap(sizeof(pkgCache::Dependency)); if (unlikely(Dependency == 0)) return false; bool isDuplicate = false; - map_pointer_t DependencyData = 0; - map_pointer_t PreviousData = 0; + map_pointer DependencyData = 0; + map_pointer PreviousData = 0; if (Pkg->RevDepends != 0) { pkgCache::Dependency const * const L = Cache.DepP + Pkg->RevDepends; @@ -1054,7 +1054,7 @@ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, for (pkgCache::DepIterator D = Ver.DependsList(); D.end() == false; ++D) OldDepLast = &D->NextDepends; } else if (oldMap != Map.Data()) - OldDepLast += static_cast(Map.Data()) - static_cast(oldMap); + OldDepLast += static_cast const *>(Map.Data()) - static_cast const *>(oldMap); Dep->NextDepends = *OldDepLast; *OldDepLast = Dependency; @@ -1171,11 +1171,11 @@ bool pkgCacheListParser::NewProvides(pkgCache::VerIterator &Ver, } bool pkgCacheGenerator::NewProvides(pkgCache::VerIterator &Ver, pkgCache::PkgIterator &Pkg, - map_pointer_t const ProvideVersion, + map_stringitem_t const ProvideVersion, uint8_t const Flags) { // Get a structure - map_pointer_t const Provides = AllocateInMap(sizeof(pkgCache::Provides)); + map_pointer const Provides = AllocateInMap(sizeof(pkgCache::Provides)); if (unlikely(Provides == 0)) return false; ++Cache.HeaderP->ProvidesCount; @@ -1249,7 +1249,7 @@ bool pkgCacheGenerator::SelectReleaseFile(const string &File,const string &Site, return true; // Get some space for the structure - map_pointer_t const idxFile = AllocateInMap(sizeof(*CurrentRlsFile)); + map_pointer const idxFile = AllocateInMap(sizeof(*CurrentRlsFile)); if (unlikely(idxFile == 0)) return false; CurrentRlsFile = Cache.RlsFileP + idxFile; @@ -1283,7 +1283,7 @@ bool pkgCacheGenerator::SelectFile(std::string const &File, { CurrentFile = nullptr; // Get some space for the structure - map_pointer_t const idxFile = AllocateInMap(sizeof(*CurrentFile)); + map_pointer const idxFile = AllocateInMap(sizeof(*CurrentFile)); if (unlikely(idxFile == 0)) return false; CurrentFile = Cache.PkgFileP + idxFile; @@ -1630,7 +1630,7 @@ static bool loadBackMMapFromFile(std::unique_ptr &Gen, if (CacheF.IsOpen() == false || CacheF.Seek(0) == false || CacheF.Failed()) return false; _error->PushToStack(); - map_pointer_t const alloc = Map->RawAllocate(CacheF.Size()); + map_pointer const alloc = Map->RawAllocate(CacheF.Size()); bool const newError = _error->PendingError(); _error->MergeWithStack(); if (alloc == 0 && newError) diff --git a/apt-pkg/pkgcachegen.h b/apt-pkg/pkgcachegen.h index 70192c28e..d088bca52 100644 --- a/apt-pkg/pkgcachegen.h +++ b/apt-pkg/pkgcachegen.h @@ -40,7 +40,7 @@ class APT_HIDDEN pkgCacheGenerator /*{{{*/ APT_HIDDEN map_stringitem_t WriteStringInMap(APT::StringView String) { return WriteStringInMap(String.data(), String.size()); }; APT_HIDDEN map_stringitem_t WriteStringInMap(const char *String); APT_HIDDEN map_stringitem_t WriteStringInMap(const char *String, const unsigned long &Len); - APT_HIDDEN map_pointer_t AllocateInMap(const unsigned long &size); + APT_HIDDEN map_pointer AllocateInMap(const unsigned long &size); // Dirty hack for public users that do not use C++11 yet #if __cplusplus >= 201103L @@ -108,15 +108,15 @@ class APT_HIDDEN pkgCacheGenerator /*{{{*/ bool NewGroup(pkgCache::GrpIterator &Grp, APT::StringView Name); bool NewPackage(pkgCache::PkgIterator &Pkg, APT::StringView Name, APT::StringView Arch); - map_pointer_t NewVersion(pkgCache::VerIterator &Ver, APT::StringView const &VerStr, - map_pointer_t const ParentPkg, uint32_t Hash, - map_pointer_t const Next); - map_pointer_t NewDescription(pkgCache::DescIterator &Desc,const std::string &Lang, APT::StringView md5sum,map_stringitem_t const idxmd5str); + map_pointer NewVersion(pkgCache::VerIterator &Ver, APT::StringView const &VerStr, + map_pointer const ParentPkg, uint32_t Hash, + map_pointer const Next); + map_pointer NewDescription(pkgCache::DescIterator &Desc,const std::string &Lang, APT::StringView md5sum,map_stringitem_t const idxmd5str); bool NewFileVer(pkgCache::VerIterator &Ver,ListParser &List); bool NewFileDesc(pkgCache::DescIterator &Desc,ListParser &List); bool NewDepends(pkgCache::PkgIterator &Pkg, pkgCache::VerIterator &Ver, - map_pointer_t const Version, uint8_t const Op, - uint8_t const Type, map_pointer_t* &OldDepLast); + map_pointer const Version, uint8_t const Op, + uint8_t const Type, map_pointer* &OldDepLast); bool NewProvides(pkgCache::VerIterator &Ver, pkgCache::PkgIterator &Pkg, map_stringitem_t const ProvidesVersion, uint8_t const Flags); @@ -172,7 +172,7 @@ class APT_HIDDEN pkgCacheListParser // Some cache items pkgCache::VerIterator OldDepVer; - map_pointer_t *OldDepLast; + map_pointer *OldDepLast; void * const d; diff --git a/apt-private/private-cachefile.cc b/apt-private/private-cachefile.cc index ab25338ff..4becc147e 100644 --- a/apt-private/private-cachefile.cc +++ b/apt-private/private-cachefile.cc @@ -22,7 +22,7 @@ using namespace std; static bool SortPackagesByName(pkgCache * const Owner, - map_pointer_t const A, map_pointer_t const B) + map_pointer const A, map_pointer const B) { if (A == 0) return false; @@ -42,7 +42,7 @@ void SortedPackageUniverse::LazyInit() const return; pkgCache * const Owner = data(); // In Multi-Arch systems Grps are easier to sort than Pkgs - std::vector GrpList; + std::vector> GrpList; List.reserve(Owner->Head().GroupCount); for (pkgCache::GrpIterator I{Owner->GrpBegin()}; I.end() != true; ++I) GrpList.emplace_back(I - Owner->GrpP); diff --git a/apt-private/private-cachefile.h b/apt-private/private-cachefile.h index 5086ceaeb..ccd47107b 100644 --- a/apt-private/private-cachefile.h +++ b/apt-private/private-cachefile.h @@ -13,7 +13,7 @@ class APT_PUBLIC CacheFile : public pkgCacheFile { public: - std::vector UniverseList; + std::vector> UniverseList; bool CheckDeps(bool AllowBroken = false); bool BuildCaches(bool WithLock = true) @@ -40,13 +40,13 @@ class APT_PUBLIC CacheFile : public pkgCacheFile class SortedPackageUniverse : public APT::PackageUniverse { - std::vector &List; + std::vector> &List; void LazyInit() const; public: explicit SortedPackageUniverse(CacheFile &Cache); - class const_iterator : public APT::Container_iterator_base::const_iterator, pkgCache::PkgIterator> + class const_iterator : public APT::Container_iterator_base>::const_iterator, pkgCache::PkgIterator> { pkgCache * const Cache; public: @@ -55,8 +55,8 @@ public: if (*_iter == 0) return pkgCache::PkgIterator(*Cache); return pkgCache::PkgIterator(*Cache, Cache->PkgP + *_iter); } - explicit const_iterator(pkgCache * const Owner, std::vector::const_iterator i): - Container_iterator_base::const_iterator, pkgCache::PkgIterator>(i), Cache(Owner) {} + explicit const_iterator(pkgCache * const Owner, std::vector>::const_iterator i): + Container_iterator_base>::const_iterator, pkgCache::PkgIterator>(i), Cache(Owner) {} }; typedef const_iterator iterator; diff --git a/cmdline/apt-cache.cc b/cmdline/apt-cache.cc index acf00bdda..2d6c1a91e 100644 --- a/cmdline/apt-cache.cc +++ b/cmdline/apt-cache.cc @@ -134,14 +134,14 @@ static bool DumpPackage(CommandLine &CmdL) // ShowHashTableStats - Show stats about a hashtable /*{{{*/ // --------------------------------------------------------------------- /* */ -static map_pointer_t PackageNext(pkgCache::Package const * const P) { return P->NextPackage; } -static map_pointer_t GroupNext(pkgCache::Group const * const G) { return G->Next; } +static map_pointer PackageNext(pkgCache::Package const * const P) { return P->NextPackage; } +static map_pointer GroupNext(pkgCache::Group const * const G) { return G->Next; } template static void ShowHashTableStats(char const *const Type, T *StartP, - map_pointer_t *Hashtable, + map_pointer *Hashtable, unsigned long Size, - map_pointer_t (*Next)(T const *const)) + map_pointer (*Next)(T const *const)) { // hashtable stats for the HashTable unsigned long NumBuckets = Size; -- cgit v1.2.3-70-g09d2 From 1f4e2ab7462f5e05e452fb8505185895d91651c2 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 24 Feb 2020 18:09:49 +0100 Subject: Wrap AllocateInMap with a templated version --- apt-pkg/pkgcachegen.cc | 30 ++++++++++++++++-------------- apt-pkg/pkgcachegen.h | 5 ++++- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 7a6ada5a2..a1e6ca745 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -226,13 +226,15 @@ map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String) { return index; } /*}}}*/ -map_pointer pkgCacheGenerator::AllocateInMap(const unsigned long &size) {/*{{{*/ +uint32_t pkgCacheGenerator::AllocateInMap(const unsigned long &size) {/*{{{*/ size_t oldSize = Map.Size(); void const * const oldMap = Map.Data(); - map_pointer const index = Map.Allocate(size); + auto index = Map.Allocate(size); if (index != 0) ReMap(oldMap, Map.Data(), oldSize); - return index; + if (index != static_cast(index)) + abort(); // Internal error + return static_cast(index); } /*}}}*/ // CacheGenerator::MergeList - Merge the package list /*{{{*/ @@ -553,7 +555,7 @@ bool pkgCacheGenerator::NewGroup(pkgCache::GrpIterator &Grp, StringView Name) return true; // Get a structure - map_pointer const Group = AllocateInMap(sizeof(pkgCache::Group)); + auto const Group = AllocateInMap(); if (unlikely(Group == 0)) return false; @@ -594,7 +596,7 @@ bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name, return true; // Get a structure - map_pointer const Package = AllocateInMap(sizeof(pkgCache::Package)); + auto const Package = AllocateInMap(); if (unlikely(Package == 0)) return false; Pkg = pkgCache::PkgIterator(Cache,Cache.PkgP + Package); @@ -820,7 +822,7 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, return true; // Get a structure - map_pointer const VerFile = AllocateInMap(sizeof(pkgCache::VerFile)); + auto const VerFile = AllocateInMap(); if (VerFile == 0) return false; @@ -853,7 +855,7 @@ map_pointer pkgCacheGenerator::NewVersion(pkgCache::VerIterat map_pointer const Next) { // Get a structure - map_pointer const Version = AllocateInMap(sizeof(pkgCache::Version)); + auto const Version = AllocateInMap(); if (Version == 0) return 0; @@ -905,7 +907,7 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, return true; // Get a structure - map_pointer const DescFile = AllocateInMap(sizeof(pkgCache::DescFile)); + auto const DescFile = AllocateInMap(); if (DescFile == 0) return false; @@ -938,7 +940,7 @@ map_pointer pkgCacheGenerator::NewDescription(pkgCache::D map_stringitem_t const idxmd5str) { // Get a structure - map_pointer const Description = AllocateInMap(sizeof(pkgCache::Description)); + auto const Description = AllocateInMap(); if (Description == 0) return 0; @@ -976,7 +978,7 @@ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, { void const * const oldMap = Map.Data(); // Get a structure - map_pointer const Dependency = AllocateInMap(sizeof(pkgCache::Dependency)); + auto const Dependency = AllocateInMap(); if (unlikely(Dependency == 0)) return false; @@ -1003,7 +1005,7 @@ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, if (isDuplicate == false) { - DependencyData = AllocateInMap(sizeof(pkgCache::DependencyData)); + DependencyData = AllocateInMap(); if (unlikely(DependencyData == 0)) return false; } @@ -1175,7 +1177,7 @@ bool pkgCacheGenerator::NewProvides(pkgCache::VerIterator &Ver, uint8_t const Flags) { // Get a structure - map_pointer const Provides = AllocateInMap(sizeof(pkgCache::Provides)); + auto const Provides = AllocateInMap(); if (unlikely(Provides == 0)) return false; ++Cache.HeaderP->ProvidesCount; @@ -1249,7 +1251,7 @@ bool pkgCacheGenerator::SelectReleaseFile(const string &File,const string &Site, return true; // Get some space for the structure - map_pointer const idxFile = AllocateInMap(sizeof(*CurrentRlsFile)); + auto const idxFile = AllocateInMap(); if (unlikely(idxFile == 0)) return false; CurrentRlsFile = Cache.RlsFileP + idxFile; @@ -1283,7 +1285,7 @@ bool pkgCacheGenerator::SelectFile(std::string const &File, { CurrentFile = nullptr; // Get some space for the structure - map_pointer const idxFile = AllocateInMap(sizeof(*CurrentFile)); + auto const idxFile = AllocateInMap(); if (unlikely(idxFile == 0)) return false; CurrentFile = Cache.PkgFileP + idxFile; diff --git a/apt-pkg/pkgcachegen.h b/apt-pkg/pkgcachegen.h index d088bca52..c042625c4 100644 --- a/apt-pkg/pkgcachegen.h +++ b/apt-pkg/pkgcachegen.h @@ -40,7 +40,10 @@ class APT_HIDDEN pkgCacheGenerator /*{{{*/ APT_HIDDEN map_stringitem_t WriteStringInMap(APT::StringView String) { return WriteStringInMap(String.data(), String.size()); }; APT_HIDDEN map_stringitem_t WriteStringInMap(const char *String); APT_HIDDEN map_stringitem_t WriteStringInMap(const char *String, const unsigned long &Len); - APT_HIDDEN map_pointer AllocateInMap(const unsigned long &size); + APT_HIDDEN uint32_t AllocateInMap(const unsigned long &size); + template map_pointer AllocateInMap() { + return map_pointer{AllocateInMap(sizeof(T))}; + } // Dirty hack for public users that do not use C++11 yet #if __cplusplus >= 201103L -- cgit v1.2.3-70-g09d2 From 4fad7262291a8af1415fb9a3693678bd9610f0d6 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Mon, 24 Feb 2020 17:46:10 +0100 Subject: Make map_pointer typesafe Instead of just using uint32_t, which would allow you to assign e.g. a map_pointer to a map_pointer, use our own smarter struct that has strict type checking. We allow creating a map_pointer from a nullptr, and we allow comparing map_pointer to nullptr, which also deals with comparisons against 0 which are often used, as 0 will be implictly converted to nullptr. --- apt-pkg/cacheiterators.h | 9 +++++---- apt-pkg/deb/deblistparser.cc | 4 ++-- apt-pkg/edsp/edsplistparser.cc | 4 ++-- apt-pkg/pkgcache.cc | 2 +- apt-pkg/pkgcache.h | 22 +++++++++++++++++++- apt-pkg/pkgcachegen.cc | 43 ++++++++++++++++++++-------------------- apt-pkg/pkgcachegen.h | 2 +- apt-private/private-cachefile.cc | 2 +- cmdline/apt-cache.cc | 4 ++-- 9 files changed, 57 insertions(+), 35 deletions(-) diff --git a/apt-pkg/cacheiterators.h b/apt-pkg/cacheiterators.h index 7f853558b..d2e4f7f90 100644 --- a/apt-pkg/cacheiterators.h +++ b/apt-pkg/cacheiterators.h @@ -80,6 +80,7 @@ template class pkgCache::Iterator : // Mixed stuff inline bool IsGood() const { return S && Owner && ! end();} inline unsigned long Index() const {return S - OwnerPointer();} + inline map_pointer MapPointer() const {return map_pointer(Index()) ;} void ReMap(void const * const oldMap, void const * const newMap) { if (Owner == 0 || S == 0) @@ -293,7 +294,7 @@ class pkgCache::DepIterator : public Iterator { inline PkgIterator TargetPkg() const {return PkgIterator(*Owner,Owner->PkgP + S2->Package);} inline PkgIterator SmartTargetPkg() const {PkgIterator R(*Owner,0);SmartTargetPkg(R);return R;} inline VerIterator ParentVer() const {return VerIterator(*Owner,Owner->VerP + S->ParentVer);} - inline PkgIterator ParentPkg() const {return PkgIterator(*Owner,Owner->PkgP + Owner->VerP[S->ParentVer].ParentPkg);} + inline PkgIterator ParentPkg() const {return PkgIterator(*Owner,Owner->PkgP + Owner->VerP[uint32_t(S->ParentVer)].ParentPkg);} inline bool Reverse() const {return Type == DepRev;} bool IsCritical() const APT_PURE; bool IsNegative() const APT_PURE; @@ -378,7 +379,7 @@ class pkgCache::PrvIterator : public Iterator { inline const char *ProvideVersion() const {return S->ProvideVersion == 0?0:Owner->StrP + S->ProvideVersion;} inline PkgIterator ParentPkg() const {return PkgIterator(*Owner,Owner->PkgP + S->ParentPkg);} inline VerIterator OwnerVer() const {return VerIterator(*Owner,Owner->VerP + S->Version);} - inline PkgIterator OwnerPkg() const {return PkgIterator(*Owner,Owner->PkgP + Owner->VerP[S->Version].ParentPkg);} + inline PkgIterator OwnerPkg() const {return PkgIterator(*Owner,Owner->PkgP + Owner->VerP[uint32_t(S->Version)].ParentPkg);} /* MultiArch can be translated to SingleArch for an resolver and we did so, by adding provides to help the resolver understand the problem, but @@ -475,7 +476,7 @@ class pkgCache::VerFileIterator : public pkgCache::IteratorFile + Owner->PkgFileP);} + inline PkgFileIterator File() const {return PkgFileIterator(*Owner, Owner->PkgFileP + S->File);} inline VerFileIterator() : Iterator() {} inline VerFileIterator(pkgCache &Owner,VerFile *Trg) : Iterator(Owner, Trg) {} @@ -493,7 +494,7 @@ class pkgCache::DescFileIterator : public Iterator { inline DescFileIterator operator++(int) { DescFileIterator const tmp(*this); operator++(); return tmp; } // Accessors - inline PkgFileIterator File() const {return PkgFileIterator(*Owner,S->File + Owner->PkgFileP);} + inline PkgFileIterator File() const {return PkgFileIterator(*Owner, Owner->PkgFileP + S->File);} inline DescFileIterator() : Iterator() {} inline DescFileIterator(pkgCache &Owner,DescFile *Trg) : Iterator(Owner, Trg) {} diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc index eaa9dfda9..ab957a01a 100644 --- a/apt-pkg/deb/deblistparser.cc +++ b/apt-pkg/deb/deblistparser.cc @@ -208,7 +208,7 @@ bool debListParser::NewVersion(pkgCache::VerIterator &Ver) // Link into by source package group. Ver->SourcePkgName = G->Name; Ver->NextInSource = G->VersionsInSource; - G->VersionsInSource = Ver.Index(); + G->VersionsInSource = Ver.MapPointer(); Ver->MultiArch = ParseMultiArch(true); // Archive Size @@ -469,7 +469,7 @@ bool debStatusListParser::ParseStatus(pkgCache::PkgIterator &Pkg, if (Ver.end() == true) _error->Warning("Encountered status field in a non-version description"); else - Pkg->CurrentVer = Ver.Index(); + Pkg->CurrentVer = Ver.MapPointer(); } return true; diff --git a/apt-pkg/edsp/edsplistparser.cc b/apt-pkg/edsp/edsplistparser.cc index 45abdbc61..34b9ec934 100644 --- a/apt-pkg/edsp/edsplistparser.cc +++ b/apt-pkg/edsp/edsplistparser.cc @@ -87,7 +87,7 @@ bool edspListParser::ParseStatus(pkgCache::PkgIterator &Pkg, if (state != 0) { Pkg->CurrentState = pkgCache::State::Installed; - Pkg->CurrentVer = Ver.Index(); + Pkg->CurrentVer = Ver.MapPointer(); } if (Section.FindB("APT-Automatic", false)) @@ -162,7 +162,7 @@ bool eippListParser::ParseStatus(pkgCache::PkgIterator &Pkg, case pkgCache::State::TriggersAwaited: case pkgCache::State::TriggersPending: case pkgCache::State::Installed: - Pkg->CurrentVer = Ver.Index(); + Pkg->CurrentVer = Ver.MapPointer(); break; } break; diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc index 59f4256ea..02448a073 100644 --- a/apt-pkg/pkgcache.cc +++ b/apt-pkg/pkgcache.cc @@ -451,7 +451,7 @@ pkgCache::PkgIterator pkgCache::GrpIterator::NextPkg(pkgCache::PkgIterator const LastPkg.end() == true)) return PkgIterator(*Owner, 0); - if (S->LastPackage == LastPkg.Index()) + if (S->LastPackage == LastPkg.MapPointer()) return PkgIterator(*Owner, 0); return PkgIterator(*Owner, Owner->PkgP + LastPkg->NextPackage); diff --git a/apt-pkg/pkgcache.h b/apt-pkg/pkgcache.h index e5a1a81eb..ef22a78a0 100644 --- a/apt-pkg/pkgcache.h +++ b/apt-pkg/pkgcache.h @@ -77,6 +77,7 @@ #include #include +#include // required for nullptr_t #include #include #include @@ -92,10 +93,29 @@ typedef uint32_t map_filesize_small_t; typedef uint32_t map_id_t; // some files get an id, too, but in far less absolute numbers typedef uint16_t map_fileid_t; + // relative pointer from cache start -template using map_pointer = uint32_t; +template class map_pointer { + uint32_t val; +public: + map_pointer() noexcept : val(0) {} + map_pointer(nullptr_t) noexcept : val(0) {} + explicit map_pointer(uint32_t n) noexcept : val(n) {} + explicit operator uint32_t() noexcept { return val; } +}; + +template inline T *operator +(T *p, map_pointer m) { return p + uint32_t(m); } +template inline bool operator ==(map_pointer u, map_pointer m) { return uint32_t(u) == uint32_t(m); } +template inline bool operator !=(map_pointer u, map_pointer m) { return uint32_t(u) != uint32_t(m); } +template inline bool operator <(map_pointer u, map_pointer m) { return uint32_t(u) < uint32_t(m); } +template inline bool operator >(map_pointer u, map_pointer m) { return uint32_t(u) > uint32_t(m); } +template inline uint32_t operator -(map_pointer u, map_pointer m) { return uint32_t(u) - uint32_t(m); } +template bool operator ==(map_pointer m, nullptr_t) { return uint32_t(m) == 0; } +template bool operator !=(map_pointer m, nullptr_t) { return uint32_t(m) != 0; } + // same as the previous, but documented to be to a string item typedef map_pointer map_stringitem_t; + // we have only a small amount of flags for each item typedef uint8_t map_flags_t; typedef uint8_t map_number_t; diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index a1e6ca745..83a8c2e63 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -210,7 +210,7 @@ map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String, const unsigned long &Len) { size_t oldSize = Map.Size(); void const * const oldMap = Map.Data(); - map_stringitem_t const index = Map.WriteString(String, Len); + map_stringitem_t const index{Map.WriteString(String, Len)}; if (index != 0) ReMap(oldMap, Map.Data(), oldSize); return index; @@ -220,7 +220,7 @@ map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String, map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String) { size_t oldSize = Map.Size(); void const * const oldMap = Map.Data(); - map_stringitem_t const index = Map.WriteString(String); + map_stringitem_t const index{Map.WriteString(String)}; if (index != 0) ReMap(oldMap, Map.Data(), oldSize); return index; @@ -375,7 +375,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator { pkgCache::VerIterator Ver = Pkg.VersionList(); Dynamic DynVer(Ver); - map_pointer *LastVer = &Pkg->VersionList; + map_pointer *LastVer = &Pkg->VersionList; void const * oldMap = Map.Data(); auto Hash = List.VersionHash(); @@ -435,7 +435,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator } // Add a new version - map_pointer const verindex = NewVersion(Ver, Version, Pkg.Index(), Hash, *LastVer); + map_pointer const verindex = NewVersion(Ver, Version, Pkg.MapPointer(), Hash, *LastVer); if (unlikely(verindex == 0)) return _error->Error(_("Error occurred while processing %s (%s%d)"), Pkg.Name(), "NewVersion", 1); @@ -527,7 +527,7 @@ bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterato Ver.ParentPkg().Name(), "NewDescription", 1); md5idx = Desc->md5sum; - Desc->ParentPkg = Ver.ParentPkg().Index(); + 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 @@ -602,7 +602,7 @@ bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name, Pkg = pkgCache::PkgIterator(Cache,Cache.PkgP + Package); // Set the name, arch and the ID - Pkg->Group = Grp.Index(); + Pkg->Group = Grp.MapPointer(); // all is mapped to the native architecture map_stringitem_t const idxArch = (Arch == "all") ? Cache.HeaderP->Architecture : StoreString(MIXED, Arch); if (unlikely(idxArch == 0)) @@ -827,14 +827,14 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, return false; pkgCache::VerFileIterator VF(Cache,Cache.VerFileP + VerFile); - VF->File = CurrentFile - Cache.PkgFileP; + VF->File = map_pointer{CurrentFile - Cache.PkgFileP}; // Link it to the end of the list map_pointer *Last = &Ver->FileList; for (pkgCache::VerFileIterator V = Ver.FileList(); V.end() == false; ++V) Last = &V->NextFile; VF->NextFile = *Last; - *Last = VF.Index(); + *Last = VF.MapPointer(); VF->Offset = List.Offset(); VF->Size = List.Size(); @@ -912,7 +912,7 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, return false; pkgCache::DescFileIterator DF(Cache,Cache.DescFileP + DescFile); - DF->File = CurrentFile - Cache.PkgFileP; + DF->File = map_pointer{CurrentFile - Cache.PkgFileP}; // Link it to the end of the list map_pointer *Last = &Desc->FileList; @@ -920,7 +920,7 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, Last = &D->NextFile; DF->NextFile = *Last; - *Last = DF.Index(); + *Last = DF.MapPointer(); DF->Offset = List.Offset(); DF->Size = List.Size(); @@ -971,7 +971,7 @@ map_pointer pkgCacheGenerator::NewDescription(pkgCache::D version and to the package that it is pointing to. */ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, pkgCache::VerIterator &Ver, - map_pointer const Version, + map_stringitem_t const Version, uint8_t const Op, uint8_t const Type, map_pointer * &OldDepLast) @@ -1011,7 +1011,7 @@ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, } pkgCache::Dependency * Link = Cache.DepP + Dependency; - Link->ParentVer = Ver.Index(); + Link->ParentVer = Ver.MapPointer(); Link->DependencyData = DependencyData; Link->ID = Cache.HeaderP->DependsCount++; @@ -1021,7 +1021,7 @@ bool pkgCacheGenerator::NewDepends(pkgCache::PkgIterator &Pkg, Dep->Type = Type; Dep->CompareOp = Op; Dep->Version = Version; - Dep->Package = Pkg.Index(); + Dep->Package = Pkg.MapPointer(); ++Cache.HeaderP->DependsDataCount; if (PreviousData != 0) { @@ -1184,16 +1184,16 @@ bool pkgCacheGenerator::NewProvides(pkgCache::VerIterator &Ver, // Fill it in pkgCache::PrvIterator Prv(Cache,Cache.ProvideP + Provides,Cache.PkgP); - Prv->Version = Ver.Index(); + Prv->Version = Ver.MapPointer(); Prv->ProvideVersion = ProvideVersion; Prv->Flags = Flags; Prv->NextPkgProv = Ver->ProvidesList; - Ver->ProvidesList = Prv.Index(); + Ver->ProvidesList = Prv.MapPointer(); // Link it to the package - Prv->ParentPkg = Pkg.Index(); + Prv->ParentPkg = Pkg.MapPointer(); Prv->NextProvides = Pkg->ProvidesList; - Pkg->ProvidesList = Prv.Index(); + Pkg->ProvidesList = Prv.MapPointer(); return true; } /*}}}*/ @@ -1267,7 +1267,7 @@ bool pkgCacheGenerator::SelectReleaseFile(const string &File,const string &Site, CurrentRlsFile->Flags = Flags; CurrentRlsFile->ID = Cache.HeaderP->ReleaseFileCount; RlsFileName = File; - Cache.HeaderP->RlsFileList = CurrentRlsFile - Cache.RlsFileP; + Cache.HeaderP->RlsFileList = map_pointer{CurrentRlsFile - Cache.RlsFileP}; Cache.HeaderP->ReleaseFileCount++; return true; @@ -1316,11 +1316,11 @@ bool pkgCacheGenerator::SelectFile(std::string const &File, CurrentFile->Component = component; CurrentFile->Flags = Flags; if (CurrentRlsFile != nullptr) - CurrentFile->Release = CurrentRlsFile - Cache.RlsFileP; + CurrentFile->Release = map_pointer{CurrentRlsFile - Cache.RlsFileP}; else CurrentFile->Release = 0; PkgFileName = File; - Cache.HeaderP->FileList = CurrentFile - Cache.PkgFileP; + Cache.HeaderP->FileList = map_pointer{CurrentFile - Cache.PkgFileP}; Cache.HeaderP->PackageFileCount++; if (Progress != 0) @@ -1362,6 +1362,7 @@ public: ScopedErrorRevert() { _error->PushToStack(); } ~ScopedErrorRevert() { _error->RevertToStack(); } }; + static bool CheckValidity(FileFd &CacheFile, std::string const &CacheFileName, pkgSourceList &List, FileIterator const Start, @@ -1632,7 +1633,7 @@ static bool loadBackMMapFromFile(std::unique_ptr &Gen, if (CacheF.IsOpen() == false || CacheF.Seek(0) == false || CacheF.Failed()) return false; _error->PushToStack(); - map_pointer const alloc = Map->RawAllocate(CacheF.Size()); + uint32_t const alloc = Map->RawAllocate(CacheF.Size()); bool const newError = _error->PendingError(); _error->MergeWithStack(); if (alloc == 0 && newError) diff --git a/apt-pkg/pkgcachegen.h b/apt-pkg/pkgcachegen.h index c042625c4..f5b4c80b3 100644 --- a/apt-pkg/pkgcachegen.h +++ b/apt-pkg/pkgcachegen.h @@ -118,7 +118,7 @@ class APT_HIDDEN pkgCacheGenerator /*{{{*/ bool NewFileVer(pkgCache::VerIterator &Ver,ListParser &List); bool NewFileDesc(pkgCache::DescIterator &Desc,ListParser &List); bool NewDepends(pkgCache::PkgIterator &Pkg, pkgCache::VerIterator &Ver, - map_pointer const Version, uint8_t const Op, + map_stringitem_t const Version, uint8_t const Op, uint8_t const Type, map_pointer* &OldDepLast); bool NewProvides(pkgCache::VerIterator &Ver, pkgCache::PkgIterator &Pkg, map_stringitem_t const ProvidesVersion, uint8_t const Flags); diff --git a/apt-private/private-cachefile.cc b/apt-private/private-cachefile.cc index 4becc147e..9d875b4a7 100644 --- a/apt-private/private-cachefile.cc +++ b/apt-private/private-cachefile.cc @@ -22,7 +22,7 @@ using namespace std; static bool SortPackagesByName(pkgCache * const Owner, - map_pointer const A, map_pointer const B) + map_pointer const A, map_pointer const B) { if (A == 0) return false; diff --git a/cmdline/apt-cache.cc b/cmdline/apt-cache.cc index 2d6c1a91e..23ab7e47f 100644 --- a/cmdline/apt-cache.cc +++ b/cmdline/apt-cache.cc @@ -467,7 +467,7 @@ static bool DumpAvail(CommandLine &) char *Buffer = new char[Cache->HeaderP->MaxVerFileSize+10]; for (pkgCache::VerFile **J = VFList; *J != 0;) { - pkgCache::PkgFileIterator File(*Cache,(*J)->File + Cache->PkgFileP); + pkgCache::PkgFileIterator File(*Cache, Cache->PkgFileP + (*J)->File); // FIXME: Add support for volatile/with-source files FileFd PkgF(File.FileName(),FileFd::ReadOnly, FileFd::Extension); if (_error->PendingError() == true) @@ -481,7 +481,7 @@ static bool DumpAvail(CommandLine &) unsigned long Pos = 0; for (; *J != 0; J++) { - if ((*J)->File + Cache->PkgFileP != File) + if (Cache->PkgFileP + (*J)->File != File) break; const pkgCache::VerFile &VF = **J; -- cgit v1.2.3-70-g09d2 From f17e3ce8213ba364d1eea7b5bab286a2175e5875 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 25 Feb 2020 11:26:18 +0100 Subject: Silence narrow conversion warnings, add error checks When converting a long offset to a uint32_t to be stored in the map, check that this is safe to do. If the offset is negative, or we lose data in the conversion, we lost. --- apt-pkg/pkgcachegen.cc | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 83a8c2e63..283866f88 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -48,6 +48,19 @@ static bool IsDuplicateDescription(pkgCache &Cache, pkgCache::DescIterator Desc, using std::string; using APT::StringView; +// Convert an offset returned from e.g. DynamicMMap or ptr difference to +// an uint32_t location without data loss. +template +static inline uint32_t NarrowOffset(T ptr) +{ + uint32_t res = static_cast(ptr); + if (unlikely(ptr < 0)) + abort(); + if (unlikely(static_cast(res) != ptr)) + abort(); + return res; +} + // CacheGenerator::pkgCacheGenerator - Constructor /*{{{*/ // --------------------------------------------------------------------- /* We set the dirty flag and make sure that is written to the disk */ @@ -210,7 +223,7 @@ map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String, const unsigned long &Len) { size_t oldSize = Map.Size(); void const * const oldMap = Map.Data(); - map_stringitem_t const index{Map.WriteString(String, Len)}; + map_stringitem_t const index{NarrowOffset(Map.WriteString(String, Len))}; if (index != 0) ReMap(oldMap, Map.Data(), oldSize); return index; @@ -220,7 +233,7 @@ map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String, map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String) { size_t oldSize = Map.Size(); void const * const oldMap = Map.Data(); - map_stringitem_t const index{Map.WriteString(String)}; + map_stringitem_t const index{NarrowOffset(Map.WriteString(String))}; if (index != 0) ReMap(oldMap, Map.Data(), oldSize); return index; @@ -827,7 +840,7 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, return false; pkgCache::VerFileIterator VF(Cache,Cache.VerFileP + VerFile); - VF->File = map_pointer{CurrentFile - Cache.PkgFileP}; + VF->File = map_pointer{NarrowOffset(CurrentFile - Cache.PkgFileP)}; // Link it to the end of the list map_pointer *Last = &Ver->FileList; @@ -912,7 +925,7 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, return false; pkgCache::DescFileIterator DF(Cache,Cache.DescFileP + DescFile); - DF->File = map_pointer{CurrentFile - Cache.PkgFileP}; + DF->File = map_pointer{NarrowOffset(CurrentFile - Cache.PkgFileP)}; // Link it to the end of the list map_pointer *Last = &Desc->FileList; @@ -1267,7 +1280,7 @@ bool pkgCacheGenerator::SelectReleaseFile(const string &File,const string &Site, CurrentRlsFile->Flags = Flags; CurrentRlsFile->ID = Cache.HeaderP->ReleaseFileCount; RlsFileName = File; - Cache.HeaderP->RlsFileList = map_pointer{CurrentRlsFile - Cache.RlsFileP}; + Cache.HeaderP->RlsFileList = map_pointer{NarrowOffset(CurrentRlsFile - Cache.RlsFileP)}; Cache.HeaderP->ReleaseFileCount++; return true; @@ -1316,11 +1329,11 @@ bool pkgCacheGenerator::SelectFile(std::string const &File, CurrentFile->Component = component; CurrentFile->Flags = Flags; if (CurrentRlsFile != nullptr) - CurrentFile->Release = map_pointer{CurrentRlsFile - Cache.RlsFileP}; + CurrentFile->Release = map_pointer{NarrowOffset(CurrentRlsFile - Cache.RlsFileP)}; else CurrentFile->Release = 0; PkgFileName = File; - Cache.HeaderP->FileList = map_pointer{CurrentFile - Cache.PkgFileP}; + Cache.HeaderP->FileList = map_pointer{NarrowOffset(CurrentFile - Cache.PkgFileP)}; Cache.HeaderP->PackageFileCount++; if (Progress != 0) -- cgit v1.2.3-70-g09d2 From d8c6ea90784ecb4dabbf8c2cb7b631add50ae177 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Tue, 25 Feb 2020 11:44:16 +0100 Subject: Add d-pointers to groups, packages, versions, and files This allows us to extend those in-cache objects with more data later on without breaking the ABI. Reserve 12 pointers for private data in the pkgCache class, and double the size of pools to 24. --- apt-pkg/pkgcache.h | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/apt-pkg/pkgcache.h b/apt-pkg/pkgcache.h index ef22a78a0..f68736ddc 100644 --- a/apt-pkg/pkgcache.h +++ b/apt-pkg/pkgcache.h @@ -230,6 +230,7 @@ class pkgCache /*{{{*/ Dependency *DepP; DependencyData *DepDataP; char *StrP; + void *reserved[12]; virtual bool ReMap(bool const &Errorchecks = true); inline bool Sync() {return Map.Sync();} @@ -365,10 +366,10 @@ struct pkgCache::Header Start indicates the first byte of the pool, Count is the number of objects remaining in the pool and ItemSize is the structure size (alignment factor) of the pool. An ItemSize of 0 indicates the pool is empty. There should be - the same number of pools as there are structure types. The generator + twice the number of pools as there are non-private structure types. The generator stores this information so future additions can make use of any unused pool blocks. */ - DynamicMMap::Pool Pools[12]; + DynamicMMap::Pool Pools[2 * 12]; /** \brief hash tables providing rapid group/package name lookup @@ -425,6 +426,8 @@ struct pkgCache::Group /** \brief List of binary produces by source package with this name. */ map_pointer VersionsInSource; + /** \brief Private pointer */ + map_pointer d; }; /*}}}*/ // Package structure /*{{{*/ @@ -487,6 +490,9 @@ struct pkgCache::Package map_id_t ID; /** \brief some useful indicators of the package's state */ map_flags_t Flags; + + /** \brief Private pointer */ + map_pointer d; }; /*}}}*/ // Release File structure /*{{{*/ @@ -527,6 +533,9 @@ struct pkgCache::ReleaseFile map_pointer NextFile; /** \brief unique sequel ID */ map_fileid_t ID; + + /** \brief Private pointer */ + map_pointer d; }; /*}}}*/ // Package File structure /*{{{*/ @@ -566,6 +575,9 @@ struct pkgCache::PackageFile map_pointer NextFile; /** \brief unique sequel ID */ map_fileid_t ID; + + /** \brief Private pointer */ + map_pointer d; }; /*}}}*/ // VerFile structure /*{{{*/ @@ -670,6 +682,9 @@ struct pkgCache::Version map_number_t Priority; /** \brief next version in the source package (might be different binary) */ map_pointer NextInSource; + + /** \brief Private pointer */ + map_pointer d; }; /*}}}*/ // Description structure /*{{{*/ -- cgit v1.2.3-70-g09d2