Restore the apt ABI.
authorDaniel Burrows <Daniel_Burrows@alumni.brown.edu>
Fri, 26 Sep 2008 01:24:09 +0000 (18:24 -0700)
committerDaniel Burrows <Daniel_Burrows@alumni.brown.edu>
Fri, 26 Sep 2008 01:24:09 +0000 (18:24 -0700)
The problem was that the size of pkgDpkgPM and its member offsets
changed because a map giving the names of the trigger states was
inserted into the middle of the structure.  I fixed it by using a
statically allocated array instead.  This changes the procedure for
looking up a string to a linear search, which should be fine (or
even faster than before) since there are only 4 state strings.  If
it becomes a problem, sorting the array by key will allow us to use
std::equal_range(), but I would advise against this unless it's
really necessary, since sooner or later someone will forget to maintain
the sort order.

apt-pkg/deb/dpkgpm.cc
apt-pkg/deb/dpkgpm.h
debian/changelog

index 0071c15..70dd09b 100644 (file)
@@ -25,6 +25,8 @@
 #include <signal.h>
 #include <errno.h>
 #include <stdio.h>
+#include <string.h>
+#include <algorithm>
 #include <sstream>
 #include <map>
 
 
 using namespace std;
 
-
+namespace
+{
+  // Maps the dpkg "processing" info to human readable names.  Entry 0
+  // of each array is the key, entry 1 is the value.
+  const std::pair<const char *, const char *> PackageProcessingOps[] = {
+    std::make_pair("install",   N_("Installing %s")),
+    std::make_pair("configure", N_("Configuring %s")),
+    std::make_pair("remove",    N_("Removing %s")),
+    std::make_pair("trigproc",  N_("Running post-installation trigger %s"))
+  };
+
+  const std::pair<const char *, const char *> * const PackageProcessingOpsBegin = PackageProcessingOps;
+  const std::pair<const char *, const char *> * const PackageProcessingOpsEnd   = PackageProcessingOps + sizeof(PackageProcessingOps) / sizeof(PackageProcessingOps[0]);
+
+  // Predicate to test whether an entry in the PackageProcessingOps
+  // array matches a string.
+  class MatchProcessingOp
+  {
+    const char *target;
+
+  public:
+    MatchProcessingOp(const char *the_target)
+      : target(the_target)
+    {
+    }
+
+    bool operator()(const std::pair<const char *, const char *> &pair) const
+    {
+      return strcmp(pair.first, target) == 0;
+    }
+  };
+}
 
 // DPkgPM::pkgDPkgPM - Constructor                                     /*{{{*/
 // ---------------------------------------------------------------------
@@ -362,17 +395,19 @@ void pkgDPkgPM::ProcessDpkgStatusLine(int OutStatusFd, char *line)
    if(strncmp(list[0], "processing", strlen("processing")) == 0)
    {
       char s[200];
-      map<string,string>::iterator iter;
       char *pkg_or_trigger = _strstrip(list[2]);
       action =_strstrip( list[1]);
-      iter = PackageProcessingOps.find(action);
-      if(iter == PackageProcessingOps.end())
+      const std::pair<const char *, const char *> * const iter =
+       std::find_if(PackageProcessingOpsBegin,
+                    PackageProcessingOpsEnd,
+                    MatchProcessingOp(action));
+      if(iter == PackageProcessingOpsEnd)
       {
         if (_config->FindB("Debug::pkgDPkgProgressReporting",false) == true)
            std::clog << "ignoring unknwon action: " << action << std::endl;
         return;
       }
-      snprintf(s, sizeof(s), _(iter->second.c_str()), pkg_or_trigger);
+      snprintf(s, sizeof(s), _(iter->second), pkg_or_trigger);
 
       status << "pmstatus:" << pkg_or_trigger
             << ":"  << (PackagesDone/float(PackagesTotal)*100.0) 
@@ -601,12 +636,6 @@ bool pkgDPkgPM::Go(int OutStatusFd)
       },
    };
 
-   // populate the "processing" map
-   PackageProcessingOps.insert( make_pair("install",N_("Installing %s")) );
-   PackageProcessingOps.insert( make_pair("configure",N_("Configuring %s")) );
-   PackageProcessingOps.insert( make_pair("remove",N_("Removing %s")) );
-   PackageProcessingOps.insert( make_pair("trigproc",N_("Running post-installation trigger %s")) );
-
    // init the PackageOps map, go over the list of packages that
    // that will be [installed|configured|removed|purged] and add
    // them to the PackageOps map (the dpkg states it goes through)
index 4494691..ebc7e32 100644 (file)
@@ -47,8 +47,6 @@ class pkgDPkgPM : public pkgPackageManager
    // the int is the state that is already done (e.g. a package that is
    // going to be install is already in state "half-installed")
    map<string,unsigned int> PackageOpsDone;
-   // map the dpkg "processing" info to human readable names
-   map<string,string> PackageProcessingOps;
    // progress reporting
    unsigned int PackagesDone;
    unsigned int PackagesTotal;
index 5c381ab..8913ef4 100644 (file)
@@ -1,3 +1,15 @@
+apt (UNRELEASED) experimental; urgency=low
+
+  [Daniel Burrows]
+  * apt-pkg/deb/dpkgpm.cc:
+    - Store the trigger state descriptions in a way that does not break
+      the ABI.  The approach taken makes the search for a string O(n) rather
+      than O(lg(n)), but since n == 4, I do not consider this a major
+      concern.  If it becomes a concern, we can sort the static array and
+      use std::equal_range().  (Closes: #499322)
+
+ --
+
 apt (0.7.15~exp2) experimental; urgency=low
 
   [ Michael Vogt ]