From 4ccd3b3ce69d6a6598e1773689a44fdec78a85cc Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Mon, 5 Aug 2013 22:40:28 +0200 Subject: fix some unitialized data members --- apt-pkg/cdrom.h | 2 +- apt-pkg/contrib/sha2.h | 4 +++- apt-pkg/tagfile.h | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apt-pkg/cdrom.h b/apt-pkg/cdrom.h index 4fc3d3928..7d19eb813 100644 --- a/apt-pkg/cdrom.h +++ b/apt-pkg/cdrom.h @@ -18,7 +18,7 @@ class pkgCdromStatus /*{{{*/ int totalSteps; public: - pkgCdromStatus() {}; + pkgCdromStatus() : totalSteps(0) {}; virtual ~pkgCdromStatus() {}; // total steps diff --git a/apt-pkg/contrib/sha2.h b/apt-pkg/contrib/sha2.h index 51c921dbd..8e0c99a1b 100644 --- a/apt-pkg/contrib/sha2.h +++ b/apt-pkg/contrib/sha2.h @@ -60,10 +60,11 @@ class SHA256Summation : public SHA2SummationBase res.Set(Sum); return res; }; - SHA256Summation() + SHA256Summation() { SHA256_Init(&ctx); Done = false; + memset(&Sum, 0, sizeof(Sum)); }; }; @@ -96,6 +97,7 @@ class SHA512Summation : public SHA2SummationBase { SHA512_Init(&ctx); Done = false; + memset(&Sum, 0, sizeof(Sum)); }; }; diff --git a/apt-pkg/tagfile.h b/apt-pkg/tagfile.h index 4718f5101..126f4219d 100644 --- a/apt-pkg/tagfile.h +++ b/apt-pkg/tagfile.h @@ -84,7 +84,7 @@ class pkgTagSection Stop = this->Stop; }; - pkgTagSection() : Section(0), TagCount(0), Stop(0) {}; + pkgTagSection() : Section(0), TagCount(0), Stop(0), d(NULL) {}; virtual ~pkgTagSection() {}; }; -- cgit v1.2.3 From 1680800209dc85387c4c2da9a1edfcde44f87807 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 7 Aug 2013 18:00:07 +0200 Subject: specific pins below 1000 cause downgrades MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We start your quest by using the version of a package applying to a specific pin, but that version could very well be below the current version, which causes APT to suggest a downgrade even if it is advertised that it never does this below 1000. Its of course questionable what use a specific pin on a package has which has a newer version already installed, but reacting with the suggestion of a downgrade is really not appropriated (even if its kinda likely that this is actually the intend the user has – it could just as well be an outdated pin) and as pinning is complicated enough we should atleast do what is described in the manpage. So we look out for the specific pin and if we haven't seen it at the moment we see the installed version, we ignore the specific pin. Closes: 543966 --- apt-pkg/policy.cc | 25 +++++-- .../test-bug-543966-downgrade-below-1000-pin | 81 ++++++++++++++++++++++ 2 files changed, 99 insertions(+), 7 deletions(-) create mode 100755 test/integration/test-bug-543966-downgrade-below-1000-pin diff --git a/apt-pkg/policy.cc b/apt-pkg/policy.cc index 4ae3b5f87..0a06cc6e3 100644 --- a/apt-pkg/policy.cc +++ b/apt-pkg/policy.cc @@ -166,11 +166,15 @@ pkgCache::VerIterator pkgPolicy::GetCandidateVer(pkgCache::PkgIterator const &Pk tracks the default when the default is taken away, and a permanent pin that stays at that setting. */ + bool PrefSeen = false; for (pkgCache::VerIterator Ver = Pkg.VersionList(); Ver.end() == false; ++Ver) { /* Lets see if this version is the installed version */ bool instVer = (Pkg.CurrentVer() == Ver); + if (Pref == Ver) + PrefSeen = true; + for (pkgCache::VerFileIterator VF = Ver.FileList(); VF.end() == false; ++VF) { /* If this is the status file, and the current version is not the @@ -187,26 +191,33 @@ pkgCache::VerIterator pkgPolicy::GetCandidateVer(pkgCache::PkgIterator const &Pk { Pref = Ver; Max = Prio; + PrefSeen = true; } if (Prio > MaxAlt) { PrefAlt = Ver; MaxAlt = Prio; - } - } - + } + } + if (instVer == true && Max < 1000) { + /* Not having seen the Pref yet means we have a specific pin below 1000 + on a version below the current installed one, so ignore the specific pin + as this would be a downgrade otherwise */ + if (PrefSeen == false || Pref.end() == true) + { + Pref = Ver; + PrefSeen = true; + } /* Elevate our current selection (or the status file itself) to the Pseudo-status priority. */ - if (Pref.end() == true) - Pref = Ver; Max = 1000; - + // Fast path optimize. if (StatusOverride == false) break; - } + } } // If we do not find our candidate, use the one with the highest pin. // This means that if there is a version available with pin > 0; there diff --git a/test/integration/test-bug-543966-downgrade-below-1000-pin b/test/integration/test-bug-543966-downgrade-below-1000-pin new file mode 100755 index 000000000..f602bea95 --- /dev/null +++ b/test/integration/test-bug-543966-downgrade-below-1000-pin @@ -0,0 +1,81 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'i386' + +insertpackage 'unstable' 'base-files' 'all' '5.0.0' +insertinstalledpackage 'base-files' 'all' '5.0.0-1' + +setupaptarchive + +STATUS=$(readlink -f rootdir/var/lib/dpkg/status) +APTARCHIVE="$(readlink -f aptarchive)/" + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Version table: + *** 5.0.0-1 0 + 100 $STATUS + 5.0.0 0 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=0 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 99' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 99 + 100 $STATUS + 5.0.0 99 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=99 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 100' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 100 + 100 $STATUS + 5.0.0 100 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=100 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 999' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0-1 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 999 + 100 $STATUS + 5.0.0 999 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=999 + +echo 'Package: base-files +Pin: release a=unstable +Pin-Priority: 1000' > rootdir/etc/apt/preferences + +testequal "base-files: + Installed: 5.0.0-1 + Candidate: 5.0.0 + Package pin: 5.0.0 + Version table: + *** 5.0.0-1 1000 + 100 $STATUS + 5.0.0 1000 + 500 file:${APTARCHIVE} unstable/main i386 Packages" aptcache policy base-files -o apt::pin=1000 -- cgit v1.2.3 From 91c4cc14d3654636edf997d23852f05ad3de4853 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 7 Aug 2013 16:40:39 +0200 Subject: stop skipping "-----" sections in Release files The file we read will always be a Release file as the clearsign is stripped earlier in this method, so this check is just wasting CPU Its also removing the risk that this could ever be part of a valid section, even if I can't imagine how that should be valid. Git-Dch: Ignore --- apt-pkg/indexrecords.cc | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/apt-pkg/indexrecords.cc b/apt-pkg/indexrecords.cc index e37a78cfb..6d89949a0 100644 --- a/apt-pkg/indexrecords.cc +++ b/apt-pkg/indexrecords.cc @@ -62,7 +62,7 @@ bool indexRecords::Load(const string Filename) /*{{{*/ if (OpenMaybeClearSignedFile(Filename, Fd) == false) return false; - pkgTagFile TagFile(&Fd, Fd.Size() + 256); // XXX + pkgTagFile TagFile(&Fd); if (_error->PendingError() == true) { strprintf(ErrorText, _("Unable to parse Release file %s"),Filename.c_str()); @@ -71,16 +71,11 @@ bool indexRecords::Load(const string Filename) /*{{{*/ pkgTagSection Section; const char *Start, *End; - // Skip over sections beginning with ----- as this is an idicator for clearsigns - do { - if (TagFile.Step(Section) == false) - { - strprintf(ErrorText, _("No sections in Release file %s"), Filename.c_str()); - return false; - } - - Section.Get (Start, End, 0); - } while (End - Start > 5 && strncmp(Start, "-----", 5) == 0); + if (TagFile.Step(Section) == false) + { + strprintf(ErrorText, _("No sections in Release file %s"), Filename.c_str()); + return false; + } Suite = Section.FindS("Suite"); Dist = Section.FindS("Codename"); -- cgit v1.2.3 From e9737c7f6a3e03b2975927ef9b04c1194026ed9c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Wed, 7 Aug 2013 16:45:49 +0200 Subject: use pkgTagFile to parse "header" of Release files The handwritten parsing here was mostly done as we couldn't trust the Release file we got, but nowadays we are sure that the Release file is valid and contains just a single section we want it to include. Beside reducing code it also fixes a bug: Fieldnames in deb822 formatted files are case-insensitive and pkgTagFile does it correctly, but this selfbuilt stuff here didn't. --- apt-pkg/deb/deblistparser.cc | 104 ++++++++----------------------------------- 1 file changed, 19 insertions(+), 85 deletions(-) diff --git a/apt-pkg/deb/deblistparser.cc b/apt-pkg/deb/deblistparser.cc index 28857176b..c2707d0a5 100644 --- a/apt-pkg/deb/deblistparser.cc +++ b/apt-pkg/deb/deblistparser.cc @@ -805,94 +805,28 @@ bool debListParser::LoadReleaseInfo(pkgCache::PkgFileIterator &FileI, map_ptrloc const storage = WriteUniqString(component); FileI->Component = storage; - // FIXME: should use FileFd and TagSection - FILE* release = fdopen(dup(File.Fd()), "r"); - if (release == NULL) + pkgTagFile TagFile(&File); + pkgTagSection Section; + if (_error->PendingError() == true || TagFile.Step(Section) == false) return false; - char buffer[101]; - while (fgets(buffer, sizeof(buffer), release) != NULL) - { - size_t len = 0; - - // Skip empty lines - for (; buffer[len] == '\r' && buffer[len] == '\n'; ++len) - /* nothing */ - ; - if (buffer[len] == '\0') - continue; - - // seperate the tag from the data - const char* dataStart = strchr(buffer + len, ':'); - if (dataStart == NULL) - continue; - len = dataStart - buffer; - for (++dataStart; *dataStart == ' '; ++dataStart) - /* nothing */ - ; - const char* dataEnd = (const char*)rawmemchr(dataStart, '\0'); - // The last char should be a newline, but we can never be sure: #633350 - const char* lineEnd = dataEnd; - for (--lineEnd; *lineEnd == '\r' || *lineEnd == '\n'; --lineEnd) - /* nothing */ - ; - ++lineEnd; - - // which datastorage need to be updated - enum { Suite, Component, Version, Origin, Codename, Label, None } writeTo = None; - if (buffer[0] == ' ') - ; - #define APT_PARSER_WRITETO(X) else if (strncmp(#X, buffer, len) == 0) writeTo = X; - APT_PARSER_WRITETO(Suite) - APT_PARSER_WRITETO(Component) - APT_PARSER_WRITETO(Version) - APT_PARSER_WRITETO(Origin) - APT_PARSER_WRITETO(Codename) - APT_PARSER_WRITETO(Label) - #undef APT_PARSER_WRITETO - #define APT_PARSER_FLAGIT(X) else if (strncmp(#X, buffer, len) == 0) \ - pkgTagSection::FindFlag(FileI->Flags, pkgCache::Flag:: X, dataStart, lineEnd); - APT_PARSER_FLAGIT(NotAutomatic) - APT_PARSER_FLAGIT(ButAutomaticUpgrades) - #undef APT_PARSER_FLAGIT - - // load all data from the line and save it - string data; - if (writeTo != None) - data.append(dataStart, dataEnd); - if (sizeof(buffer) - 1 == (dataEnd - buffer)) - { - while (fgets(buffer, sizeof(buffer), release) != NULL) - { - if (writeTo != None) - data.append(buffer); - if (strlen(buffer) != sizeof(buffer) - 1) - break; - } - } - if (writeTo != None) - { - // remove spaces and stuff from the end of the data line - for (std::string::reverse_iterator s = data.rbegin(); - s != data.rend(); ++s) - { - if (*s != '\r' && *s != '\n' && *s != ' ') - break; - *s = '\0'; - } - map_ptrloc const storage = WriteUniqString(data); - switch (writeTo) { - case Suite: FileI->Archive = storage; break; - case Component: FileI->Component = storage; break; - case Version: FileI->Version = storage; break; - case Origin: FileI->Origin = storage; break; - case Codename: FileI->Codename = storage; break; - case Label: FileI->Label = storage; break; - case None: break; - } - } + std::string data; + #define APT_INRELEASE(TAG, STORE) \ + data = Section.FindS(TAG); \ + if (data.empty() == false) \ + { \ + map_ptrloc const storage = WriteUniqString(data); \ + STORE = storage; \ } - fclose(release); + APT_INRELEASE("Suite", FileI->Archive) + APT_INRELEASE("Component", FileI->Component) + APT_INRELEASE("Version", FileI->Version) + APT_INRELEASE("Origin", FileI->Origin) + APT_INRELEASE("Codename", FileI->Codename) + APT_INRELEASE("Label", FileI->Label) + #undef APT_INRELEASE + Section.FindFlag("NotAutomatic", FileI->Flags, pkgCache::Flag::NotAutomatic); + Section.FindFlag("ButAutomaticUpgrades", FileI->Flags, pkgCache::Flag::ButAutomaticUpgrades); return !_error->PendingError(); } -- cgit v1.2.3 From f52037d629aea696f938015e7f1ec037eb079af8 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 8 Aug 2013 15:40:15 +0200 Subject: fix -Wall errors --- apt-pkg/deb/dpkgpm.cc | 6 ++++-- apt-pkg/tagfile.h | 2 +- apt-pkg/vendorlist.cc | 2 +- cmdline/apt-cdrom.cc | 3 ++- ftparchive/writer.cc | 3 ++- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index b0bd6b184..34ae4e593 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -134,7 +134,8 @@ static void dpkgChrootDirectory() std::cerr << "Chrooting into " << chrootDir << std::endl; if (chroot(chrootDir.c_str()) != 0) _exit(100); - chdir("/"); + if (chdir("/") != 0) + _exit(100); } /*}}}*/ @@ -755,7 +756,8 @@ bool pkgDPkgPM::OpenLog() pw = getpwnam("root"); gr = getgrnam("adm"); if (pw != NULL && gr != NULL) - chown(logfile_name.c_str(), pw->pw_uid, gr->gr_gid); + if(chown(logfile_name.c_str(), pw->pw_uid, gr->gr_gid) != 0) + _error->Errno("OpenLog", "chown failed"); chmod(logfile_name.c_str(), 0640); fprintf(d->term_out, "\nLog started: %s\n", timestr); } diff --git a/apt-pkg/tagfile.h b/apt-pkg/tagfile.h index 126f4219d..fedd72701 100644 --- a/apt-pkg/tagfile.h +++ b/apt-pkg/tagfile.h @@ -84,7 +84,7 @@ class pkgTagSection Stop = this->Stop; }; - pkgTagSection() : Section(0), TagCount(0), Stop(0), d(NULL) {}; + pkgTagSection() : Section(0), TagCount(0), d(NULL), Stop(0) {}; virtual ~pkgTagSection() {}; }; diff --git a/apt-pkg/vendorlist.cc b/apt-pkg/vendorlist.cc index ecfc7db87..602425624 100644 --- a/apt-pkg/vendorlist.cc +++ b/apt-pkg/vendorlist.cc @@ -66,7 +66,7 @@ bool pkgVendorList::CreateList(Configuration& Cnf) /*{{{*/ Configuration Block(Top); string VendorID = Top->Tag; vector *Fingerprints = new vector; - struct Vendor::Fingerprint *Fingerprint = new struct Vendor::Fingerprint; + struct Vendor::Fingerprint *Fingerprint = new struct Vendor::Fingerprint(); string Origin = Block.Find("Origin"); Fingerprint->Print = Block.Find("Fingerprint"); diff --git a/cmdline/apt-cdrom.cc b/cmdline/apt-cdrom.cc index c153cca85..545edf439 100644 --- a/cmdline/apt-cdrom.cc +++ b/cmdline/apt-cdrom.cc @@ -66,7 +66,8 @@ void pkgCdromTextStatus::Prompt(const char *Text) { char C; cout << Text << ' ' << flush; - read(STDIN_FILENO,&C,1); + if (read(STDIN_FILENO,&C,1) < 0) + _error->Errno("pkgCdromTextStatus::Prompt", "failed to prompt"); if (C != '\n') cout << endl; } diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc index 3283128d8..7ecfe78ed 100644 --- a/ftparchive/writer.cc +++ b/ftparchive/writer.cc @@ -284,7 +284,8 @@ bool FTWScanner::Delink(string &FileName,const char *OriginalPath, if (link(FileName.c_str(),OriginalPath) != 0) { // Panic! Restore the symlink - symlink(OldLink,OriginalPath); + if (symlink(OldLink,OriginalPath) != 0) + _error->Errno("symlink", "failed to restore symlink"); return _error->Errno("link",_("*** Failed to link %s to %s"), FileName.c_str(), OriginalPath); -- cgit v1.2.3