fix item updater freeSpace()

Fixed freeSpace() to remove more than 1 version if that is what is
needed to get the number of active PNOR versions at
ACTIVE_PNOR_MAX_ALLOWED -1. Currently, ACTIVE_PNOR_MAX_ALLOWED is
set to 2. We have seen cases where there are 3 active PNOR versions.
In those cases, freeSpace() only removes 1 active PNOR version, when
it should remove 2 to bring the total number of active PNOR
versions to 1.

Resolves openbmc/openbmc#2806

Change-Id: I0e9f5b6835298ae86091848f889bbc316e0a7f57
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
diff --git a/item_updater.cpp b/item_updater.cpp
index becff46..1d4f3ce 100644
--- a/item_updater.cpp
+++ b/item_updater.cpp
@@ -1,6 +1,7 @@
 #include <string>
 #include <experimental/filesystem>
 #include <fstream>
+#include <queue>
 #include <phosphor-logging/elog-errors.hpp>
 #include <phosphor-logging/log.hpp>
 #include "xyz/openbmc_project/Common/error.hpp"
@@ -546,31 +547,36 @@
 // TODO: openbmc/openbmc#1402 Monitor flash usage
 void ItemUpdater::freeSpace()
 {
+    //  Versions with the highest priority in front
+    std::priority_queue<std::pair<int, std::string>,
+                        std::vector<std::pair<int, std::string>>,
+                        std::less<std::pair<int, std::string>>> versionsPQ;
+
     std::size_t count = 0;
-    decltype(activations.begin()->second->redundancyPriority.get()->priority())
-            highestPriority = 0;
-    decltype(activations.begin()->second->versionId) highestPriorityVersion;
     for (const auto& iter : activations)
     {
         if (iter.second.get()->activation() == server::Activation::Activations::Active)
         {
             count++;
+            // Don't put the functional version on the queue since we can't
+            // remove the "running" PNOR version.
             if (isVersionFunctional(iter.second->versionId))
             {
                 continue;
             }
-            if (iter.second->redundancyPriority.get()->priority() >= highestPriority)
-            {
-                highestPriority = iter.second->redundancyPriority.get()->priority();
-                highestPriorityVersion = iter.second->versionId;
-            }
+            versionsPQ.push(std::make_pair(
+                    iter.second->redundancyPriority.get()->priority(),
+                    iter.second->versionId));
         }
     }
-    // Remove the pnor version with highest priority since the PNOR
-    // can't hold more than 2 versions.
-    if (count >= ACTIVE_PNOR_MAX_ALLOWED)
+
+    // If the number of PNOR versions is over ACTIVE_PNOR_MAX_ALLOWED -1,
+    // remove the highest priority one(s).
+    while ((count >= ACTIVE_PNOR_MAX_ALLOWED) && (!versionsPQ.empty()))
     {
-        erase(highestPriorityVersion);
+        erase(versionsPQ.top().second);
+        versionsPQ.pop();
+        count--;
     }
 }
 
diff --git a/item_updater.hpp b/item_updater.hpp
index f09891c..7b37e4d 100755
--- a/item_updater.hpp
+++ b/item_updater.hpp
@@ -141,8 +141,12 @@
          */
         void deleteAll();
 
-        /** @brief Deletes the active PNOR version with highest priority
-                   if the total number of volumes exceeds the threshold.
+        /** @brief Brings the total number of active PNOR versions to
+         *         ACTIVE_PNOR_MAX_ALLOWED -1. This function is intended to be
+         *         run before activating a new PNOR version. If this function
+         *         needs to delete any PNOR version(s) it will delete the
+         *         version(s) with the highest priority, skipping the
+         *         functional PNOR version.
          */
         void freeSpace();