[PATCH] don't pass compile-time constants as an argument to DynamicMMap::Allocate()

[Ivan Zakharyaschev]

The argument was used for the size of the item we want to allocate.

Instead of an argument, use a parameter for the template DynamicMMap::Allocate()
which denotes the type of the item.

Now, the truth of our assertion that the size is not zero is evident
(and checked by the compiler).

Note that this change may also potentially lead to more optimized code
due to compile-time specialization for each particular size.
---
 apt/apt-pkg/contrib/mmap.h | 12 ++++++------
 apt/apt-pkg/pkgcachegen.cc | 19 ++++++++++---------
 apt/apt-pkg/pkgcachegen.h  |  3 ++-
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/apt/apt-pkg/contrib/mmap.h b/apt/apt-pkg/contrib/mmap.h
index 5b14ac9d9..7ec0fb8b1 100644
--- a/apt/apt-pkg/contrib/mmap.h
+++ b/apt/apt-pkg/contrib/mmap.h
@@ -94,7 +94,8 @@ class DynamicMMap : public MMap
 
    // Allocation
    std::experimental::optional<unsigned long> RawAllocate(unsigned long Size,unsigned long Aln = 0);
-   std::experimental::optional<unsigned long> Allocate(unsigned long ItemSize);
+   template<typename T>
+   std::experimental::optional<unsigned long> Allocate();
    std::experimental::optional<unsigned long> WriteString(const char *String,unsigned long Len = std::numeric_limits<unsigned long>::max());
    inline std::experimental::optional<unsigned long> WriteString(const string &S) {return WriteString(S.c_str(),S.length());};
    void UsePools(Pool &P,unsigned int Count) {Pools = &P; PoolCount = Count;};
@@ -107,17 +108,16 @@ class DynamicMMap : public MMap
 /* Templates */
 
 #include <apt-pkg/error.h>
-#include <cassert>
 
 // DynamicMMap::Allocate - Pooled aligned allocation			/*{{{*/
 // ---------------------------------------------------------------------
 /* This allocates an Item of size ItemSize so that it is aligned to its
    size in the file. */
-std::experimental::optional<unsigned long> DynamicMMap::Allocate(unsigned long ItemSize)
+template<typename T>
+std::experimental::optional<unsigned long> DynamicMMap::Allocate()
 {
-   assert(ItemSize != 0); /* Actually, we are always called with sizeof(...)
-                             compile-time non-zero constant as the argument.
-                          */
+   constexpr unsigned long ItemSize = sizeof(T);
+   static_assert(ItemSize != 0);
 
    // Look for a matching pool entry
    Pool *I;
diff --git a/apt/apt-pkg/pkgcachegen.cc b/apt/apt-pkg/pkgcachegen.cc
index 4bbab07bf..2a75f31e6 100644
--- a/apt/apt-pkg/pkgcachegen.cc
+++ b/apt/apt-pkg/pkgcachegen.cc
@@ -112,8 +112,9 @@ std::experimental::optional<unsigned long> pkgCacheGenerator::WriteStringInMap(c
    return Map.WriteString(String);
 }
 									/*}}}*/
-std::experimental::optional<unsigned long> pkgCacheGenerator::AllocateInMap(unsigned long size) {/*{{{*/
-   return Map.Allocate(size);
+template<typename T>
+std::experimental::optional<unsigned long> pkgCacheGenerator::AllocateInMap() {/*{{{*/
+   return Map.Allocate<T>();
 }
 									/*}}}*/
 // CacheGenerator::MergeList - Merge the package list			/*{{{*/
@@ -376,7 +377,7 @@ bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, const string &Nam
 #endif
        
    // Get a structure
-   const auto Package = AllocateInMap(sizeof(pkgCache::Package));
+   const auto Package = AllocateInMap<pkgCache::Package>();
    const auto idxName = WriteStringInMap(Name);
    if ((!Package) || (!idxName))
       return false;
@@ -429,7 +430,7 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver,
       return true;
    
    // Get a structure
-   const auto VerFile = AllocateInMap(sizeof(pkgCache::VerFile));
+   const auto VerFile = AllocateInMap<pkgCache::VerFile>();
    if (!VerFile)
       return false;
    
@@ -460,7 +461,7 @@ std::experimental::optional<unsigned long> pkgCacheGenerator::NewVersion(pkgCach
 					    unsigned long Next)
 {
    // Get a structure
-   const auto Version = AllocateInMap(sizeof(pkgCache::Version));
+   const auto Version = AllocateInMap<pkgCache::Version>();
    const auto idxVerStr = WriteStringInMap(VerStr);
    if ((!Version) || (!idxVerStr))
       return std::experimental::nullopt;
@@ -487,7 +488,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator &Ver,
    pkgCache &Cache = Owner->Cache;
    
    // Get a structure
-   const auto Dependency = Owner->AllocateInMap(sizeof(pkgCache::Dependency));
+   const auto Dependency = Owner->AllocateInMap<pkgCache::Dependency>();
    if (!Dependency)
       return false;
    
@@ -574,7 +575,7 @@ bool pkgCacheGenerator::ListParser::NewProvides(pkgCache::VerIterator &Ver,
       }
    }
    // Get a structure
-   const auto Provides = Owner->AllocateInMap(sizeof(pkgCache::Provides));
+   const auto Provides = Owner->AllocateInMap<pkgCache::Provides>();
    if (!Provides)
       return false;
    /* Note:
@@ -618,7 +619,7 @@ bool pkgCacheGenerator::SelectFile(const string &File, const string &Site,
 				   unsigned long Flags)
 {
    // Get some space for the structure
-   const auto idxFile = AllocateInMap(sizeof(*CurrentFile));
+   const auto idxFile = AllocateInMap<decltype(*CurrentFile)>();
    const auto idxFileName = WriteStringInMap(File);
    const auto idxSite = WriteUniqString(Site);
    const auto idxIndexType = WriteUniqString(Index.GetType()->Label);
@@ -676,7 +677,7 @@ std::experimental::optional<unsigned long> pkgCacheGenerator::WriteUniqString(co
    }
    
    // Get a structure
-   const auto Item = AllocateInMap(sizeof(pkgCache::StringItem));
+   const auto Item = AllocateInMap<pkgCache::StringItem>();
    const auto idxString = WriteStringInMap(S, Size);
    if ((!Item) || (!idxString))
       return std::experimental::nullopt;
diff --git a/apt/apt-pkg/pkgcachegen.h b/apt/apt-pkg/pkgcachegen.h
index b789177ef..73252802b 100644
--- a/apt/apt-pkg/pkgcachegen.h
+++ b/apt/apt-pkg/pkgcachegen.h
@@ -39,7 +39,8 @@ class pkgCacheGenerator
    std::experimental::optional<unsigned long> WriteStringInMap(const std::string &String) { return WriteStringInMap(String.c_str(), String.length()); };
    std::experimental::optional<unsigned long> WriteStringInMap(const char *String);
    std::experimental::optional<unsigned long> WriteStringInMap(const char *String, unsigned long Len);
-   std::experimental::optional<unsigned long> AllocateInMap(unsigned long size);
+   template<typename T>
+   std::experimental::optional<unsigned long> AllocateInMap();
 
    public:
    
-- 
2.21.0

--1807885841-580517805-1581928896=:6363--