summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJay Freeman (saurik) <saurik@saurik.com>2017-01-29 15:01:00 -0800
committerSam Bingner <sam@bingner.com>2020-07-17 00:22:23 -1000
commitbf52d8261295b37473df85c4d15130350ddb6f2a (patch)
treed89125003addce619c37586c9b9b364bc56f9a9d
parentd1fc58d91e2ef47c58adb19ac2c0c541f4bae45c (diff)
The entire concept of PendingError() is flawed :/.
-rw-r--r--apt-pkg/cachefile.cc18
-rw-r--r--apt-pkg/contrib/error.cc9
-rw-r--r--apt-pkg/contrib/error.h20
-rw-r--r--apt-pkg/deb/debindexfile.cc6
-rw-r--r--apt-pkg/deb/deblistparser.cc2
-rw-r--r--apt-pkg/indexfile.cc2
-rw-r--r--apt-pkg/pkgcachegen.cc54
-rw-r--r--apt-pkg/policy.cc3
8 files changed, 70 insertions, 44 deletions
diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc
index 9a1a6cfa9..8168a9af3 100644
--- a/apt-pkg/cachefile.cc
+++ b/apt-pkg/cachefile.cc
@@ -97,7 +97,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();
@@ -112,7 +112,7 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock)
}
if (_error->PendingError() == true)
- return false;
+ return _error->ReturnError();
if (BuildSourceList(Progress) == false)
return false;
@@ -128,14 +128,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();
@@ -169,7 +163,7 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/)
Policy.reset(new pkgPolicy(Cache));
if (_error->PendingError() == true)
- return false;
+ return _error->ReturnError();
ReadPinFile(*Policy);
ReadPinDir(*Policy);
@@ -195,7 +189,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;
@@ -219,8 +213,6 @@ bool pkgCacheFile::Open(OpProgress *Progress, bool WithLock)
if (Progress != NULL)
Progress->Done();
- if (_error->PendingError() == true)
- return false;
return true;
}
@@ -266,7 +258,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 3c397eaf8..143b99557 100644
--- a/apt-pkg/contrib/error.cc
+++ b/apt-pkg/contrib/error.cc
@@ -206,6 +206,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 d0f450742..bab7a67b2 100644
--- a/apt-pkg/contrib/error.h
+++ b/apt-pkg/contrib/error.h
@@ -226,6 +226,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 25e0a3312..78e3eecb7 100644
--- a/apt-pkg/deb/debindexfile.cc
+++ b/apt-pkg/deb/debindexfile.cc
@@ -130,7 +130,7 @@ pkgCacheListParser * debTranslationsIndex::CreateListParser(FileFd &Pkg)
std::unique_ptr<pkgCacheListParser> Parser(new debTranslationsParser(&Pkg));
bool const newError = _error->PendingError();
_error->MergeWithStack();
- return newError ? nullptr : Parser.release();
+ return newError ? ((void)_error->ReturnError(), nullptr) : Parser.release();
}
/*}}}*/
// dpkg/status Index /*{{{*/
@@ -158,7 +158,7 @@ pkgCacheListParser * debStatusIndex::CreateListParser(FileFd &Pkg)
std::unique_ptr<pkgCacheListParser> Parser(new debStatusListParser(&Pkg));
bool const newError = _error->PendingError();
_error->MergeWithStack();
- return newError ? nullptr : Parser.release();
+ return newError ? ((void)_error->ReturnError(), nullptr) : Parser.release();
}
/*}}}*/
// DebPkgFile Index - a single .deb file as an index /*{{{*/
@@ -228,7 +228,7 @@ pkgCacheListParser * debDebPkgFileIndex::CreateListParser(FileFd &Pkg)
std::unique_ptr<pkgCacheListParser> Parser(new debDebFileParser(&Pkg, DebFile));
bool const newError = _error->PendingError();
_error->MergeWithStack();
- return newError ? nullptr : Parser.release();
+ return newError ? ((void)_error->ReturnError(), nullptr) : Parser.release();
}
uint8_t debDebPkgFileIndex::GetIndexFlags() const
{
diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc
index 68c3498c8..b737a9207 100644
--- a/apt-pkg/deb/deblistparser.cc
+++ b/apt-pkg/deb/deblistparser.cc
@@ -86,7 +86,7 @@ string debListParser::Package() {
}
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 7574283f0..85d0e87cd 100644
--- a/apt-pkg/indexfile.cc
+++ b/apt-pkg/indexfile.cc
@@ -374,7 +374,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 8b3f8550d..de724d7d5 100644
--- a/apt-pkg/pkgcachegen.cc
+++ b/apt-pkg/pkgcachegen.cc
@@ -68,7 +68,7 @@ bool pkgCacheGenerator::Start()
bool const newError = _error->PendingError();
_error->MergeWithStack();
if (newError)
- return false;
+ return _error->ReturnError();
if (Map.Size() <= 0)
return false;
@@ -132,7 +132,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;
@@ -1435,7 +1433,7 @@ static bool CheckValidity(FileFd &CacheFile, std::string const &CacheFileName,
{
if (Debug == true)
std::clog << "Errors are pending or Map is empty() for " << CacheFileName << std::endl;
- return false;
+ return _error->ReturnError();
}
std::unique_ptr<bool[]> RlsVisited(new bool[Cache.HeaderP->ReleaseFileCount]);
@@ -1515,7 +1513,7 @@ static bool CheckValidity(FileFd &CacheFile, std::string const &CacheFileName,
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)
@@ -1578,8 +1576,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)
@@ -1593,8 +1593,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)
@@ -1665,7 +1667,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));
@@ -1853,13 +1855,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 7986aa506..614539266 100644
--- a/apt-pkg/policy.cc
+++ b/apt-pkg/policy.cc
@@ -333,7 +333,7 @@ bool ReadPinDir(pkgPolicy &Plcy,string Dir)
bool const PendingErrors = _error->PendingError();
_error->MergeWithStack();
if (PendingErrors)
- return false;
+ return _error->ReturnError();
// Read the files
bool good = true;
@@ -415,6 +415,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(),