implement operator* for cacheset iterators

Technically an abi-break as we change a template parameter to
std::iterator for this, but this class is empty in all instances and
just causes the right typedefs to be set – which were incorrect as
detected by std::stable_partition as its implementation uses ::pointer
and needs also a operator* implementation.

In practice CacheSets have no external users (yet) and the difference is
visible only at compile time (which was an error before and now works),
not while linking.

The changes to apt-mark are functionally identical to the code before,
just that we use a std:: algorithm now instead of trying hard on our
own.

2 files changed, 58 insertions, 40 deletions

diff --git a/apt-pkg/cacheset.h b/apt-pkg/cacheset.h
index ca0294545..47961d36b 100644
--- a/apt-pkg/cacheset.h
+++ b/apt-pkg/cacheset.h
@@ -212,7 +212,7 @@ private:
 };									/*}}}*/
 // Iterator templates for our Containers				/*{{{*/
 template<typename Interface, typename Master, typename iterator_type, typename container_iterator, typename container_value> class Container_iterator_base :
-   public std::iterator<typename std::iterator_traits<container_iterator>::iterator_category, container_iterator>,
+   public std::iterator<typename std::iterator_traits<container_iterator>::iterator_category, container_value>,
    public Interface::template iterator_base<iterator_type>
 {
 protected:
@@ -263,6 +263,7 @@ public:
 	operator typename Master::const_iterator() { return typename Master::const_iterator(this->_iter); }
 	inline iterator_type& operator=(iterator_type const &i) { this->_iter = i._iter; return static_cast<iterator_type&>(*this); }
 	inline iterator_type& operator=(container_iterator const &i) { this->_iter = i; return static_cast<iterator_type&>(*this); }
+	inline typename Container::iterator::reference operator*(void) const { return *this->_iter; }
 
 	inline typename Container::value_type getType(void) const { return *this->_iter; }
 };
@@ -289,6 +290,7 @@ public:
 	operator typename Master::const_iterator() { return typename Master::const_iterator(this->_iter); }
 	inline iterator_type& operator=(iterator_type const &i) { this->_iter = i._iter; return static_cast<iterator_type&>(*this); }
 	inline iterator_type& operator=(container_iterator const &i) { this->_iter = i; return static_cast<iterator_type&>(*this); }
+	inline typename Container::reverse_iterator::reference operator*(void) const { return *this->_iter; }
 
 	inline typename Container::value_type getType(void) const { return *this->_iter; }
 };
@@ -399,6 +401,11 @@ public:									/*{{{*/
 	typedef Container_iterator<PackageContainerInterface, Container, PackageContainer> iterator;
 	typedef Container_const_reverse_iterator<PackageContainerInterface, Container, PackageContainer> const_reverse_iterator;
 	typedef Container_reverse_iterator<PackageContainerInterface, Container, PackageContainer> reverse_iterator;
+	typedef typename Container::value_type value_type;
+	typedef typename Container::reference reference;
+	typedef typename Container::const_reference const_reference;
+	typedef typename Container::difference_type difference_type;
+	typedef typename Container::size_type size_type;
 
 	bool insert(pkgCache::PkgIterator const &P) APT_OVERRIDE { if (P.end() == true) return false; _cont.insert(P); return true; }
 	template<class Cont> void insert(PackageContainer<Cont> const &pkgcont) { _cont.insert((typename Cont::const_iterator)pkgcont.begin(), (typename Cont::const_iterator)pkgcont.end()); }
@@ -695,6 +702,12 @@ public:
 	   inline pkgCache::PkgIterator getType(void) const { return _iter; }
 	};
 	typedef const_iterator iterator;
+	typedef pkgCache::PkgIterator value_type;
+	typedef const pkgCache::PkgIterator& const_reference;
+	typedef const_reference reference;
+	typedef const_iterator::difference_type difference_type;
+	typedef std::make_unsigned<const_iterator::difference_type>::type size_type;
+
 
 	bool empty() const APT_OVERRIDE { return false; }
 	size_t size() const APT_OVERRIDE { return _cont->Head().PackageCount; }
@@ -886,10 +899,16 @@ template<class Container> class VersionContainer : public VersionContainerInterf
     pkgCache. */
 	Container _cont;
 public:									/*{{{*/
+
 	typedef Container_const_iterator<VersionContainerInterface, Container, VersionContainer> const_iterator;
 	typedef Container_iterator<VersionContainerInterface, Container, VersionContainer> iterator;
 	typedef Container_const_reverse_iterator<VersionContainerInterface, Container, VersionContainer> const_reverse_iterator;
 	typedef Container_reverse_iterator<VersionContainerInterface, Container, VersionContainer> reverse_iterator;
+	typedef typename Container::value_type value_type;
+	typedef typename Container::reference reference;
+	typedef typename Container::const_reference const_reference;
+	typedef typename Container::difference_type difference_type;
+	typedef typename Container::size_type size_type;
 
 	bool insert(pkgCache::VerIterator const &V) APT_OVERRIDE { if (V.end() == true) return false; _cont.insert(V); return true; }
 	template<class Cont> void insert(VersionContainer<Cont> const &vercont) { _cont.insert((typename Cont::const_iterator)vercont.begin(), (typename Cont::const_iterator)vercont.end()); }
diff --git a/cmdline/apt-mark.cc b/cmdline/apt-mark.cc
index 20166b312..9b51345a3 100644
--- a/cmdline/apt-mark.cc
+++ b/cmdline/apt-mark.cc
@@ -174,6 +174,41 @@ static bool DoHold(CommandLine &CmdL)
    if (unlikely(Cache == NULL))
       return false;
 
+   APT::PackageList pkgset = APT::PackageList::FromCommandLine(CacheFile, CmdL.FileList + 1);
+   if (pkgset.empty() == true)
+      return _error->Error(_("No packages found"));
+
+   bool const MarkHold = strcasecmp(CmdL.FileList[0],"hold") == 0;
+
+   auto part = std::stable_partition(pkgset.begin(), pkgset.end(),
+        [](pkgCache::PkgIterator const &P) { return P->SelectedState == pkgCache::State::Hold; });
+
+   auto doneBegin = MarkHold ? pkgset.begin() : part;
+   auto doneEnd = MarkHold ? part : pkgset.end();
+   auto changeBegin = MarkHold ? part : pkgset.begin();
+   auto changeEnd = MarkHold ? pkgset.end() : part;
+
+   std::for_each(doneBegin, doneEnd, [&MarkHold](pkgCache::PkgIterator const &P) {
+      if (MarkHold == true)
+        ioprintf(c1out, _("%s was already set on hold.\n"), P.FullName(true).c_str());
+      else
+        ioprintf(c1out, _("%s was already not hold.\n"), P.FullName(true).c_str());
+   });
+
+   if (doneBegin == pkgset.begin() && doneEnd == pkgset.end())
+      return true;
+
+   if (_config->FindB("APT::Mark::Simulate", false) == true)
+   {
+      std::for_each(changeBegin, changeEnd, [&MarkHold](pkgCache::PkgIterator const &P) {
+        if (MarkHold == false)
+           ioprintf(c1out, _("%s set on hold.\n"), P.FullName(true).c_str());
+        else
+           ioprintf(c1out, _("Canceled hold on %s.\n"), P.FullName(true).c_str());
+      });
+      return true;
+   }
+
    // Generate the base argument list for dpkg
    std::vector<const char *> Args;
    string Tmp = _config->Find("Dir::Bin::dpkg","dpkg");
@@ -201,42 +236,6 @@ static bool DoHold(CommandLine &CmdL)
 	 Args.push_back(Opts->Value.c_str());
       }
    }
-   size_t const BaseArgs = Args.size();
-
-   APT::PackageList pkgset = APT::PackageList::FromCommandLine(CacheFile, CmdL.FileList + 1);
-   if (pkgset.empty() == true)
-      return _error->Error(_("No packages found"));
-
-   bool const MarkHold = strcasecmp(CmdL.FileList[0],"hold") == 0;
-
-   for (APT::PackageList::iterator Pkg = pkgset.begin(); Pkg != pkgset.end();)
-   {
-      if ((Pkg->SelectedState == pkgCache::State::Hold) == MarkHold)
-      {
-	 if (MarkHold == true)
-	    ioprintf(c1out,_("%s was already set on hold.\n"), Pkg.FullName(true).c_str());
-	 else
-	    ioprintf(c1out,_("%s was already not hold.\n"), Pkg.FullName(true).c_str());
-	 Pkg = pkgset.erase(Pkg);
-      }
-      else
-	 ++Pkg;
-   }
-
-   if (pkgset.empty() == true)
-      return true;
-
-   if (_config->FindB("APT::Mark::Simulate", false) == true)
-   {
-      for (APT::PackageList::iterator Pkg = pkgset.begin(); Pkg != pkgset.end(); ++Pkg)
-      {
-	 if (MarkHold == false)
-	    ioprintf(c1out,_("%s set on hold.\n"), Pkg.FullName(true).c_str());
-	 else
-	    ioprintf(c1out,_("Canceled hold on %s.\n"), Pkg.FullName(true).c_str());
-      }
-      return true;
-   }
 
    APT::PackageList keepoffset;
    for (APT::PackageList::iterator Pkg = pkgset.begin(); Pkg != pkgset.end(); ++Pkg)
@@ -248,7 +247,7 @@ static bool DoHold(CommandLine &CmdL)
 
    if (keepoffset.empty() == false)
    {
-      Args.erase(Args.begin() + BaseArgs, Args.end());
+      size_t const BaseArgs = Args.size();
       Args.push_back("--merge-avail");
       // FIXME: supported only since 1.17.7 in dpkg
       Args.push_back("-");
@@ -300,9 +299,9 @@ static bool DoHold(CommandLine &CmdL)
 	 if (WIFEXITED(Status) == false || WEXITSTATUS(Status) != 0)
 	    return _error->Error(_("Executing dpkg failed. Are you root?"));
       }
+      Args.erase(Args.begin() + BaseArgs, Args.end());
    }
 
-   Args.erase(Args.begin() + BaseArgs, Args.end());
    Args.push_back("--set-selections");
    Args.push_back(NULL);
 
@@ -325,7 +324,7 @@ static bool DoHold(CommandLine &CmdL)
 
    bool const dpkgMultiArch = _system->MultiArchSupported();
    FILE* dpkg = fdopen(external[1], "w");
-   for (APT::PackageList::iterator Pkg = pkgset.begin(); Pkg != pkgset.end(); ++Pkg)
+   for (auto Pkg = changeBegin; Pkg != changeEnd; ++Pkg)
    {
       if (dpkgMultiArch == false)
 	 fprintf(dpkg, "%s", Pkg.FullName(true).c_str());