diff options
author | Julian Andres Klode <julian.klode@canonical.com> | 2020-02-24 17:46:10 +0100 |
---|---|---|
committer | Julian Andres Klode <julian.klode@canonical.com> | 2020-02-24 18:29:07 +0100 |
commit | 4fad7262291a8af1415fb9a3693678bd9610f0d6 (patch) | |
tree | 39226545753b5f0910547747fb5c691398c7341a /apt-pkg | |
parent | 1f4e2ab7462f5e05e452fb8505185895d91651c2 (diff) |
Make map_pointer<T> typesafe
Instead of just using uint32_t, which would allow you to
assign e.g. a map_pointer<Version> to a map_pointer<Package>,
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.
Diffstat (limited to 'apt-pkg')
-rw-r--r-- | apt-pkg/cacheiterators.h | 9 | ||||
-rw-r--r-- | apt-pkg/deb/deblistparser.cc | 4 | ||||
-rw-r--r-- | apt-pkg/edsp/edsplistparser.cc | 4 | ||||
-rw-r--r-- | apt-pkg/pkgcache.cc | 2 | ||||
-rw-r--r-- | apt-pkg/pkgcache.h | 22 | ||||
-rw-r--r-- | apt-pkg/pkgcachegen.cc | 43 | ||||
-rw-r--r-- | apt-pkg/pkgcachegen.h | 2 |
7 files changed, 54 insertions, 32 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<typename Str, typename Itr> class pkgCache::Iterator : // Mixed stuff inline bool IsGood() const { return S && Owner && ! end();} inline unsigned long Index() const {return S - OwnerPointer();} + inline map_pointer<Str> MapPointer() const {return map_pointer<Str>(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<Dependency, DepIterator> { 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<Provides, PrvIterator> { 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::Iterator<VerFile, VerFileIter inline VerFileIterator operator++(int) { VerFileIterator 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 VerFileIterator() : Iterator<VerFile, VerFileIterator>() {} inline VerFileIterator(pkgCache &Owner,VerFile *Trg) : Iterator<VerFile, VerFileIterator>(Owner, Trg) {} @@ -493,7 +494,7 @@ class pkgCache::DescFileIterator : public Iterator<DescFile, DescFileIterator> { 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<DescFile, DescFileIterator>() {} inline DescFileIterator(pkgCache &Owner,DescFile *Trg) : Iterator<DescFile, DescFileIterator>(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 <apt-pkg/macros.h> #include <apt-pkg/mmap.h> +#include <cstddef> // required for nullptr_t #include <string> #include <stdint.h> #include <time.h> @@ -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 <typename T> using map_pointer = uint32_t; +template <typename T> 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<typename T> inline T *operator +(T *p, map_pointer<T> m) { return p + uint32_t(m); } +template<typename T> inline bool operator ==(map_pointer<T> u, map_pointer<T> m) { return uint32_t(u) == uint32_t(m); } +template<typename T> inline bool operator !=(map_pointer<T> u, map_pointer<T> m) { return uint32_t(u) != uint32_t(m); } +template<typename T> inline bool operator <(map_pointer<T> u, map_pointer<T> m) { return uint32_t(u) < uint32_t(m); } +template<typename T> inline bool operator >(map_pointer<T> u, map_pointer<T> m) { return uint32_t(u) > uint32_t(m); } +template<typename T> inline uint32_t operator -(map_pointer<T> u, map_pointer<T> m) { return uint32_t(u) - uint32_t(m); } +template<typename T> bool operator ==(map_pointer<T> m, nullptr_t) { return uint32_t(m) == 0; } +template<typename T> bool operator !=(map_pointer<T> m, nullptr_t) { return uint32_t(m) != 0; } + // same as the previous, but documented to be to a string item typedef map_pointer<char> 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<pkgCache::VerIterator> DynVer(Ver); - map_pointer<void> *LastVer = &Pkg->VersionList; + map_pointer<pkgCache::Version> *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<pkgCache::Version> const verindex = NewVersion(Ver, Version, Pkg.Index(), Hash, *LastVer); + map_pointer<pkgCache::Version> 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<pkgCache::PackageFile>{CurrentFile - Cache.PkgFileP}; // Link it to the end of the list map_pointer<pkgCache::VerFile> *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<pkgCache::PackageFile>{CurrentFile - Cache.PkgFileP}; // Link it to the end of the list map_pointer<pkgCache::DescFile> *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<pkgCache::Description> 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<pkgCache::Version> const Version, + map_stringitem_t const Version, uint8_t const Op, uint8_t const Type, map_pointer<pkgCache::Dependency> * &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<pkgCache::ReleaseFile>{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<pkgCache::ReleaseFile>{CurrentRlsFile - Cache.RlsFileP}; else CurrentFile->Release = 0; PkgFileName = File; - Cache.HeaderP->FileList = CurrentFile - Cache.PkgFileP; + Cache.HeaderP->FileList = map_pointer<pkgCache::PackageFile>{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<pkgCacheGenerator> &Gen, if (CacheF.IsOpen() == false || CacheF.Seek(0) == false || CacheF.Failed()) return false; _error->PushToStack(); - map_pointer<void> 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<pkgCache::Version> const Version, uint8_t const Op, + map_stringitem_t const Version, uint8_t const Op, uint8_t const Type, map_pointer<pkgCache::Dependency>* &OldDepLast); bool NewProvides(pkgCache::VerIterator &Ver, pkgCache::PkgIterator &Pkg, map_stringitem_t const ProvidesVersion, uint8_t const Flags); |