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.

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<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);
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<pkgCache::Package> const A, map_pointer<pkgCache::Package> const B)
+      map_pointer<pkgCache::Group> const A, map_pointer<pkgCache::Group> 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;