The entire concept of PendingError() is flawed :/.

diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc
index b5f32fc293cb2493619cce8326ca3cbd28bd0ea0..90f803ad69c7e1265f6cfa488e458950da3b14b5 100644 (file)
--- a/apt-pkg/cachefile.cc
+++ b/apt-pkg/cachefile.cc
@@ -90,7 +90,7 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
         return false;
       Cache.reset(new pkgCache(Map.get()));
       if (_error->PendingError() == true)
-        return false;
+        return _error->ReturnError();
 
       this->Cache = Cache.release();
       this->Map = Map.release();
@@ -102,7 +102,7 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
         return false;
 
    if (_error->PendingError() == true)
-      return false;
+      return _error->ReturnError();
 
    if (BuildSourceList(Progress) == false)
       return false;
@@ -118,14 +118,8 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
    if (Res == false)
       return _error->Error(_("The package lists or status file could not be parsed or opened."));
 
-   /* This sux, remove it someday */
-   if (_error->PendingError() == true)
-      _error->Warning(_("You may want to run apt-get update to correct these problems"));
-
    if (Cache == nullptr)
       Cache.reset(new pkgCache(Map.get()));
-   if (_error->PendingError() == true)
-      return false;
    this->Map = Map.release();
    this->Cache = Cache.release();
 
@@ -159,7 +153,7 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
 
    Policy.reset(new pkgPolicy(Cache));
    if (_error->PendingError() == true)
-      return false;
+      return _error->ReturnError();
 
    if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false)
       return false;
@@ -185,7 +179,7 @@ bool pkgCacheFile::BuildDepCache(OpProgress *Progress)
 
    DCache.reset(new pkgDepCache(Cache,Policy));
    if (_error->PendingError() == true)
-      return false;
+      return _error->ReturnError();
    if (DCache->Init(Progress) == false)
       return false;
 
@@ -209,8 +203,6 @@ bool pkgCacheFile::Open(OpProgress *Progress, bool WithLock)
 
    if (Progress != NULL)
       Progress->Done();
-   if (_error->PendingError() == true)
-      return false;
    
    return true;
 }
@@ -256,7 +248,7 @@ bool pkgCacheFile::AddIndexFile(pkgIndexFile * const File)          /*{{{*/
         if (_error->PendingError() == true) {
            delete Cache;
            Cache = nullptr;
-           return false;
+           return _error->ReturnError();
         }
         return true;
       }

diff --git a/apt-pkg/contrib/error.cc b/apt-pkg/contrib/error.cc
index c06ea8364992f3ebf8a4c52d7b0b2119d49b43f8..7d397d2c696b8de2610fccc5f1dc335fdfcdb527 100644 (file)
--- a/apt-pkg/contrib/error.cc
+++ b/apt-pkg/contrib/error.cc
@@ -227,6 +227,15 @@ void GlobalError::Discard() {
        PendingFlag = false;
 }
                                                                        /*}}}*/
+// GlobalError::ReturnError - convert a stored error to a return code          /*{{{*/
+bool GlobalError::ReturnError() {
+       for (auto &message : Messages)
+               if (message.Type == ERROR)
+                       message.Type = WARNING;
+       PendingFlag = false;
+       return false;
+}
+                                                                       /*}}}*/
 // GlobalError::empty - does our error list include anything?          /*{{{*/
 bool GlobalError::empty(MsgType const &threshold) const {
        if (PendingFlag == true)

diff --git a/apt-pkg/contrib/error.h b/apt-pkg/contrib/error.h
index e56999b141ed269cf43c5ff9ba874985e0e67574..bcaa7c99592fe7545c60be39f0be51449a7a7639 100644 (file)
--- a/apt-pkg/contrib/error.h
+++ b/apt-pkg/contrib/error.h
@@ -227,6 +227,26 @@ public:                                                                    /*{{{*/
         */
        inline bool PendingError() const APT_PURE {return PendingFlag;};
 
+       /** \brief convert a stored error to a return code
+        *
+        *  Put simply, the entire concept of PendingError() is flawed :/.
+         *
+         *  The typical "if (PendingError()) return false;" check that is
+         *  strewn throughout the codebase "compounds", making it impossible
+         *  for there to be any nuance about the notion of "error" when a
+         *  subsystem needs to fail but a higher-level system needs to work.
+         *
+         *  However, the codebase is also horribly broken with respect to
+         *  errors, as it fails to use C++ exceptions when warranted and
+         *  instead relies on this insane indirect error mechanism to check
+         *  the failure status of a constructor. What is thereby needed is
+         *  a way to clear the PendingError() flag without also discarding
+         *  the underlying errors, so we have to convert them to warnings.
+         *
+        *  \return \b false
+        */
+       bool ReturnError() APT_COLD;
+
        /** \brief is the list empty?
         *
         *  Can be used to check if the current stack level doesn't include

diff --git a/apt-pkg/deb/debindexfile.cc b/apt-pkg/deb/debindexfile.cc
index c55847305ff96c4038c64eae425a6b173f0d28e2..6b162372d39e648d5a9b5a9d60bc992b8e3df749 100644 (file)
--- a/apt-pkg/deb/debindexfile.cc
+++ b/apt-pkg/deb/debindexfile.cc
@@ -134,6 +134,7 @@ pkgCacheListParser * debTranslationsIndex::CreateListParser(FileFd &Pkg)
    if (newError)
    {
       delete Parser;
+      _error->ReturnError();
       return nullptr;
    }
    else
@@ -168,6 +169,7 @@ pkgCacheListParser * debStatusIndex::CreateListParser(FileFd &Pkg)
    if (newError)
    {
       delete Parser;
+      _error->ReturnError();
       return nullptr;
    }
    else
@@ -250,6 +252,7 @@ pkgCacheListParser * debDebPkgFileIndex::CreateListParser(FileFd &Pkg)
    if (newError)
    {
       delete Parser;
+      _error->ReturnError();
       return nullptr;
    }
    else

diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc
index 25d8e6f22493578b6ed14166989424c54272e671..23048008bdbe6c42ba7439e73154884c2c9599d2 100644 (file)
--- a/apt-pkg/deb/deblistparser.cc
+++ b/apt-pkg/deb/deblistparser.cc
@@ -76,7 +76,7 @@ string debListParser::Package() {
    std::transform(Result.begin(), Result.end(), Result.begin(), tolower_ascii);
 
    if(unlikely(Result.empty() == true))
-      _error->Error("Encountered a section with no Package: header");
+      _error->Warning("Encountered a section with no Package: header");
    return Result;
 }
                                                                        /*}}}*/

diff --git a/apt-pkg/indexfile.cc b/apt-pkg/indexfile.cc
index 74d46d699fc6d353597353e59668c05a146d9d4a..21765388fc4bd7658f67057cd4d054d615feea50 100644 (file)
--- a/apt-pkg/indexfile.cc
+++ b/apt-pkg/indexfile.cc
@@ -344,6 +344,7 @@ pkgCacheListParser * pkgDebianIndexFile::CreateListParser(FileFd &Pkg)
    if (newError)
    {
       delete Parser;
+      _error->ReturnError();
       return nullptr;
    }
    else
@@ -377,7 +378,7 @@ bool pkgDebianIndexFile::Merge(pkgCacheGenerator &Gen,OpProgress * const Prog)
    File->mtime = Pkg.ModificationTime();
 
    if (Gen.MergeList(*Parser) == false)
-      return _error->Warning("Problem with MergeList %s",PackageFile.c_str());
+      return _error->Error("Problem with MergeList %s",PackageFile.c_str());
    return true;
 }
 pkgCache::PkgFileIterator pkgDebianIndexFile::FindInCache(pkgCache &Cache) const

diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc
index 51bb9ec7b3545fb97b479549169e02f2c7c666df..8ee682db83f38a7e2e9cee8f8a43047d6bbef095 100644 (file)
--- a/apt-pkg/pkgcachegen.cc
+++ b/apt-pkg/pkgcachegen.cc
@@ -70,7 +70,7 @@ bool pkgCacheGenerator::Start()
       bool const newError = _error->PendingError();
       _error->MergeWithStack();
       if (newError)
-        return false;
+        return _error->ReturnError();
       if (Map.Size() <= 0)
         return false;
 
@@ -134,7 +134,7 @@ bool pkgCacheGenerator::Start()
    advoid a problem during a crash */
 pkgCacheGenerator::~pkgCacheGenerator()
 {
-   if (_error->PendingError() == true || Map.validData() == false)
+   if (Map.validData() == false)
       return;
    if (Map.Sync() == false)
       return;
@@ -249,10 +249,8 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
    while (List.Step() == true)
    {
       string const PackageName = List.Package();
-      if (PackageName.empty() == true) {
-          _error->Warning("Encountered a section with no Package: header");
+      if (PackageName.empty() == true)
           continue;
-      }
 
       Counter++;
       if (Counter % 100 == 0 && Progress != 0)
@@ -276,7 +274,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
       if (NewPackage(Pkg, PackageName, Arch) == false) {
         // TRANSLATOR: The first placeholder is a package name,
         // the other two should be copied verbatim as they include debug info
-        _error->Warning(_("Error occurred while processing %s (%s%d)"),
+        _error->Error(_("Error occurred while processing %s (%s%d)"),
                              PackageName.c_str(), "NewPackage", 1);
          continue;
       }
@@ -340,7 +338,7 @@ bool pkgCacheGenerator::MergeListPackage(ListParser &List, pkgCache::PkgIterator
    pkgCache::VerIterator Ver(Cache);
    Dynamic<pkgCache::VerIterator> DynVer(Ver);
    if (List.UsePackage(Pkg, Ver) == false)
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
                           Pkg.Name(), "UsePackage", 1);
 
    // Find the right version to write the description
@@ -419,11 +417,11 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
       if (Res == 0 && Ver.end() == false && Ver->Hash == Hash)
       {
         if (List.UsePackage(Pkg,Ver) == false)
-           return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+           return _error->Error(_("Error occurred while processing %s (%s%d)"),
                                 Pkg.Name(), "UsePackage", 2);
 
         if (NewFileVer(Ver,List) == false)
-           return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+           return _error->Error(_("Error occurred while processing %s (%s%d)"),
                                 Pkg.Name(), "NewFileVer", 1);
 
         // Read only a single record and return
@@ -440,7 +438,7 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
    // Add a new version
    map_pointer_t const verindex = NewVersion(Ver, Version, Pkg.Index(), Hash, *LastVer);
    if (unlikely(verindex == 0))
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
                           Pkg.Name(), "NewVersion", 1);
 
    if (oldMap != Map.Data())
@@ -448,15 +446,15 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
    *LastVer = verindex;
 
    if (unlikely(List.NewVersion(Ver) == false))
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
                           Pkg.Name(), "NewVersion", 2);
 
    if (unlikely(List.UsePackage(Pkg,Ver) == false))
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
                           Pkg.Name(), "UsePackage", 3);
 
    if (unlikely(NewFileVer(Ver,List) == false))
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
                           Pkg.Name(), "NewFileVer", 2);
 
    pkgCache::GrpIterator Grp = Pkg.Group();
@@ -477,12 +475,12 @@ bool pkgCacheGenerator::MergeListVersion(ListParser &List, pkgCache::PkgIterator
         Dynamic<pkgCache::VerIterator> DynV(V);
         for (; V.end() != true; ++V)
            if (unlikely(AddImplicitDepends(V, Pkg) == false))
-              return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+              return _error->Error(_("Error occurred while processing %s (%s%d)"),
                                    Pkg.Name(), "AddImplicitDepends", 1);
       }
    }
    if (unlikely(AddImplicitDepends(Grp, Pkg, Ver) == false))
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
                           Pkg.Name(), "AddImplicitDepends", 2);
 
    // Read only a single record and return
@@ -526,7 +524,7 @@ bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterato
 
    map_pointer_t const descindex = NewDescription(Desc, lang, CurMd5, md5idx);
    if (unlikely(descindex == 0))
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
            Ver.ParentPkg().Name(), "NewDescription", 1);
 
    md5idx = Desc->md5sum;
@@ -540,7 +538,7 @@ bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterato
    *LastNextDesc = descindex;
 
    if (NewFileDesc(Desc,List) == false)
-      return _error->Warning(_("Error occurred while processing %s (%s%d)"),
+      return _error->Error(_("Error occurred while processing %s (%s%d)"),
            Ver.ParentPkg().Name(), "NewFileDesc", 1);
 
    return true;
@@ -1432,7 +1430,7 @@ static bool CheckValidity(const string &CacheFile,
    {
       if (Debug == true)
         std::clog << "Errors are pending or Map is empty() for " << CacheFile << std::endl;
-      return false;
+      return _error->ReturnError();
    }
 
    std::unique_ptr<bool[]> RlsVisited(new bool[Cache.HeaderP->ReleaseFileCount]);
@@ -1512,7 +1510,7 @@ static bool CheckValidity(const string &CacheFile,
         std::clog << "Validity failed because of pending errors:" << std::endl;
         _error->DumpErrors(std::clog, GlobalError::DEBUG, false);
       }
-      return false;
+      return _error->ReturnError();
    }
 
    if (OutMap != 0)
@@ -1575,8 +1573,10 @@ static void BuildCache(pkgCacheGenerator &Gen,
         Progress->OverallProgress(CurrentSize, TotalSize, Size, _("Reading package lists"));
       CurrentSize += Size;
 
-      if (I->Merge(Gen,Progress) == false)
+      if (I->Merge(Gen,Progress) == false) {
+        _error->ReturnError();
         return;
+      }
    };
 
    if (List !=  NULL)
@@ -1590,8 +1590,10 @@ static void BuildCache(pkgCacheGenerator &Gen,
            continue;
         }
 
-        if ((*i)->Merge(Gen, Progress) == false)
+        if ((*i)->Merge(Gen, Progress) == false) {
+           _error->ReturnError();
            continue;
+        }
 
         std::vector <pkgIndexFile *> *Indexes = (*i)->GetIndexFiles();
         if (Indexes != NULL)
@@ -1663,7 +1665,7 @@ static bool loadBackMMapFromFile(std::unique_ptr<pkgCacheGenerator> &Gen,
    bool const newError = _error->PendingError();
    _error->MergeWithStack();
    if (alloc == 0 && newError)
-      return false;
+      return _error->ReturnError();
    if (CacheF.Read((unsigned char *)Map->Data() + alloc, CacheF.Size()) == false)
       return false;
    Gen.reset(new pkgCacheGenerator(Map.get(),Progress));
@@ -1849,13 +1851,15 @@ bool pkgCacheGenerator::MakeOnlyStatusCache(OpProgress *Progress,DynamicMMap **O
    if (Progress != NULL)
       Progress->OverallProgress(0,1,1,_("Reading package lists"));
    pkgCacheGenerator Gen(Map.get(),Progress);
-   if (Gen.Start() == false || _error->PendingError() == true)
+   if (Gen.Start() == false)
       return false;
+   if (_error->PendingError() == true)
+      return _error->ReturnError();
    BuildCache(Gen,Progress,CurrentSize,TotalSize, NULL,
                  Files.begin(), Files.end());
+   // We've passed the point of no return
+   _error->ReturnError();
 
-   if (_error->PendingError() == true)
-      return false;
    *OutMap = Map.release();
    
    return true;

diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc
index 3dd6ddac4798621e9194f86ca30fb771336284cb..a13510e66782ca4dcb695d1b9c556f46ba22b260 100644 (file)
--- a/apt-pkg/policy.cc
+++ b/apt-pkg/policy.cc
@@ -320,7 +320,7 @@ bool ReadPinDir(pkgPolicy &Plcy,string Dir)
    bool const PendingErrors = _error->PendingError();
    _error->MergeWithStack();
    if (PendingErrors)
-      return false;
+      return _error->ReturnError();
 
    // Read the files
    for (vector<string>::const_iterator I = List.begin(); I != List.end(); ++I)
@@ -391,6 +391,7 @@ bool ReadPinFile(pkgPolicy &Plcy,string File)
       if (priority < std::numeric_limits<short>::min() ||
           priority > std::numeric_limits<short>::max() ||
          newError) {
+        _error->ReturnError();
         return _error->Error(_("%s: Value %s is outside the range of valid pin priorities (%d to %d)"),
                              File.c_str(), Tags.FindS("Pin-Priority").c_str(),
                              std::numeric_limits<short>::min(),