From f22b65b47990237bd5d9a1c171919c3059fbd9b0 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 9 Apr 2014 10:12:10 +0200 Subject: Fix insecure file permissions when using FileFd with OpenMode::Atomic Commit 7335eebea6dd43581d4650a8818b06383ab89901 introduced a bug that caused FileFd to create insecure permissions when FileFd::Atomic is used. This commit fixes the permissions and adds a test. The bug is most likely caused by the confusing "Perm" parameter that is passed to Open() - its not the file permissions but intead the "mode" part of open/creat. --- apt-pkg/contrib/fileutl.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 188bb87ee..8b57e87a3 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1067,6 +1067,10 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co if_FLAGGED_SET(Exclusive, O_EXCL); #undef if_FLAGGED_SET + // there is no getumask() so we read it by setting it and reset + mode_t current_umask = umask(0); + umask(current_umask); + if ((Mode & Atomic) == Atomic) { char *name = strdup((FileName + ".XXXXXX").c_str()); @@ -1080,11 +1084,11 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co TemporaryFileName = string(name); free(name); - if(Perms != 600 && fchmod(iFd, Perms) == -1) + if(Perms != 600 && fchmod(iFd, Perms & ~current_umask) == -1) return FileFdErrno("fchmod", "Could not change permissions for temporary file %s", TemporaryFileName.c_str()); } else - iFd = open(FileName.c_str(), fileflags, Perms); + iFd = open(FileName.c_str(), fileflags, Perms & ~current_umask); this->FileName = FileName; if (iFd == -1 || OpenInternDescriptor(Mode, compressor) == false) -- cgit v1.2.3-70-g09d2 From e5f3f8c101b772a6eb6326203a2174e809ab406d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 9 Apr 2014 10:24:47 +0200 Subject: Rename FileFd::Open() Perms to AccessMode Bug lp:#1304657 was caused by confusion around the name Perms. The new name AccessMode should make it clear that its not the literal file permissions but instead the AccessMode passed to open() (i.e. the umask needs to be applied) --- apt-pkg/contrib/fileutl.cc | 12 ++++++------ apt-pkg/contrib/fileutl.h | 16 ++++++++-------- 2 files changed, 14 insertions(+), 14 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 8b57e87a3..c6072ca15 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -958,10 +958,10 @@ class FileFdPrivate { /*{{{*/ // FileFd::Open - Open a file /*{{{*/ // --------------------------------------------------------------------- /* The most commonly used open mode combinations are given with Mode */ -bool FileFd::Open(string FileName,unsigned int const Mode,CompressMode Compress, unsigned long const Perms) +bool FileFd::Open(string FileName,unsigned int const Mode,CompressMode Compress, unsigned long const AccessMode) { if (Mode == ReadOnlyGzip) - return Open(FileName, ReadOnly, Gzip, Perms); + return Open(FileName, ReadOnly, Gzip, AccessMode); if (Compress == Auto && (Mode & WriteOnly) == WriteOnly) return FileFdError("Autodetection on %s only works in ReadOnly openmode!", FileName.c_str()); @@ -1028,9 +1028,9 @@ bool FileFd::Open(string FileName,unsigned int const Mode,CompressMode Compress, if (compressor == compressors.end()) return FileFdError("Can't find a match for specified compressor mode for file %s", FileName.c_str()); - return Open(FileName, Mode, *compressor, Perms); + return Open(FileName, Mode, *compressor, AccessMode); } -bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Compressor const &compressor, unsigned long const Perms) +bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Compressor const &compressor, unsigned long const AccessMode) { Close(); Flags = AutoClose; @@ -1084,11 +1084,11 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co TemporaryFileName = string(name); free(name); - if(Perms != 600 && fchmod(iFd, Perms & ~current_umask) == -1) + if(AccessMode != 600 && fchmod(iFd, AccessMode & ~current_umask) == -1) return FileFdErrno("fchmod", "Could not change permissions for temporary file %s", TemporaryFileName.c_str()); } else - iFd = open(FileName.c_str(), fileflags, Perms & ~current_umask); + iFd = open(FileName.c_str(), fileflags, AccessMode & ~current_umask); this->FileName = FileName; if (iFd == -1 || OpenInternDescriptor(Mode, compressor) == false) diff --git a/apt-pkg/contrib/fileutl.h b/apt-pkg/contrib/fileutl.h index f25ed3622..cc1a98eae 100644 --- a/apt-pkg/contrib/fileutl.h +++ b/apt-pkg/contrib/fileutl.h @@ -103,10 +103,10 @@ class FileFd return T; } - bool Open(std::string FileName,unsigned int const Mode,CompressMode Compress,unsigned long const Perms = 0666); - bool Open(std::string FileName,unsigned int const Mode,APT::Configuration::Compressor const &compressor,unsigned long const Perms = 0666); - inline bool Open(std::string const &FileName,unsigned int const Mode, unsigned long const Perms = 0666) { - return Open(FileName, Mode, None, Perms); + bool Open(std::string FileName,unsigned int const Mode,CompressMode Compress,unsigned long const AccessMode = 0666); + bool Open(std::string FileName,unsigned int const Mode,APT::Configuration::Compressor const &compressor,unsigned long const AccessMode = 0666); + inline bool Open(std::string const &FileName,unsigned int const Mode, unsigned long const AccessMode = 0666) { + return Open(FileName, Mode, None, AccessMode); }; bool OpenDescriptor(int Fd, unsigned int const Mode, CompressMode Compress, bool AutoClose=false); bool OpenDescriptor(int Fd, unsigned int const Mode, APT::Configuration::Compressor const &compressor, bool AutoClose=false); @@ -129,13 +129,13 @@ class FileFd inline bool IsCompressed() {return (Flags & Compressed) == Compressed;}; inline std::string &Name() {return FileName;}; - FileFd(std::string FileName,unsigned int const Mode,unsigned long Perms = 0666) : iFd(-1), Flags(0), d(NULL) + FileFd(std::string FileName,unsigned int const Mode,unsigned long AccessMode = 0666) : iFd(-1), Flags(0), d(NULL) { - Open(FileName,Mode, None, Perms); + Open(FileName,Mode, None, AccessMode); }; - FileFd(std::string FileName,unsigned int const Mode, CompressMode Compress, unsigned long Perms = 0666) : iFd(-1), Flags(0), d(NULL) + FileFd(std::string FileName,unsigned int const Mode, CompressMode Compress, unsigned long AccessMode = 0666) : iFd(-1), Flags(0), d(NULL) { - Open(FileName,Mode, Compress, Perms); + Open(FileName,Mode, Compress, AccessMode); }; FileFd() : iFd(-1), Flags(AutoClose), d(NULL) {}; FileFd(int const Fd, unsigned int const Mode = ReadWrite, CompressMode Compress = None) : iFd(-1), Flags(0), d(NULL) -- cgit v1.2.3-70-g09d2 From db5bf949ed796395d281474c49033f882e73f3a9 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Thu, 10 Apr 2014 09:11:57 +0200 Subject: improve umask/fchmod code readability --- apt-pkg/contrib/fileutl.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index c6072ca15..69a675648 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1067,9 +1067,12 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co if_FLAGGED_SET(Exclusive, O_EXCL); #undef if_FLAGGED_SET - // there is no getumask() so we read it by setting it and reset - mode_t current_umask = umask(0); - umask(current_umask); + // umask() will always set the umask and return the previous value, so + // we first set the umask and then reset it to the old value + mode_t CurrentUmask = umask(0); + umask(CurrentUmask); + // calculate the actual file permissions (just like open/creat) + mode_t FilePermissions = (AccessMode & ~CurrentUmask); if ((Mode & Atomic) == Atomic) { @@ -1084,11 +1087,11 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co TemporaryFileName = string(name); free(name); - if(AccessMode != 600 && fchmod(iFd, AccessMode & ~current_umask) == -1) + if(FilePermissions != 600 && fchmod(iFd, FilePermissions) == -1) return FileFdErrno("fchmod", "Could not change permissions for temporary file %s", TemporaryFileName.c_str()); } else - iFd = open(FileName.c_str(), fileflags, AccessMode & ~current_umask); + iFd = open(FileName.c_str(), fileflags, FilePermissions); this->FileName = FileName; if (iFd == -1 || OpenInternDescriptor(Mode, compressor) == false) -- cgit v1.2.3-70-g09d2 From 53c3a8fa16351f35b7b7d359c81dfbe36ab04444 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Sun, 23 Mar 2014 13:17:24 +0100 Subject: use wildcard to get files in our library makefiles The explicit listing is a pain every time you want to add a file to the list and serves no propose as we list all files there anyway, so this is not only easier but also documents this fact. Git-Dch: Ignore --- apt-inst/makefile | 12 ++---------- apt-pkg/makefile | 48 +++--------------------------------------------- apt-private/makefile | 12 ++---------- 3 files changed, 7 insertions(+), 65 deletions(-) (limited to 'apt-pkg') diff --git a/apt-inst/makefile b/apt-inst/makefile index da983df5f..af887bba8 100644 --- a/apt-inst/makefile +++ b/apt-inst/makefile @@ -20,15 +20,7 @@ SLIBS=$(PTHREADLIB) -lapt-pkg APT_DOMAIN:=libapt-inst$(MAJOR) LIBRARYDEPENDS=$(LIB)/libapt-pkg.so -# Source code for the contributed non-core things -SOURCE = contrib/extracttar.cc contrib/arfile.cc +SOURCE = $(wildcard *.cc */*.cc) +HEADERS = $(addprefix apt-pkg/,$(notdir $(wildcard *.h */*.h))) -# Source code for the main library -SOURCE+= filelist.cc dirstream.cc extract.cc deb/debfile.cc - -# Public header files -HEADERS = extracttar.h arfile.h filelist.h extract.h \ - dirstream.h debfile.h - -HEADERS := $(addprefix apt-pkg/,$(HEADERS)) include $(LIBRARY_H) diff --git a/apt-pkg/makefile b/apt-pkg/makefile index 1d456873b..5603b51ed 100644 --- a/apt-pkg/makefile +++ b/apt-pkg/makefile @@ -11,6 +11,7 @@ include ../buildlib/defaults.mak # The library name and version (indirectly used from init.h) include ../buildlib/libversion.mak + LIBRARY=apt-pkg MAJOR=$(LIBAPTPKG_MAJOR) MINOR=$(LIBAPTPKG_RELEASE) @@ -26,50 +27,7 @@ SLIBS+= -llzma endif APT_DOMAIN:=libapt-pkg$(LIBAPTPKG_MAJOR) -# Source code for the contributed non-core things -SOURCE = contrib/mmap.cc contrib/error.cc contrib/strutl.cc \ - contrib/configuration.cc contrib/progress.cc contrib/cmndline.cc \ - contrib/hashsum.cc contrib/md5.cc contrib/sha1.cc \ - contrib/sha2_internal.cc contrib/hashes.cc \ - contrib/cdromutl.cc contrib/crc-16.cc contrib/netrc.cc \ - contrib/fileutl.cc contrib/gpgv.cc -HEADERS = mmap.h error.h configuration.h fileutl.h cmndline.h netrc.h \ - md5.h crc-16.h cdromutl.h strutl.h sptr.h sha1.h sha2.h sha256.h \ - sha2_internal.h hashes.h hashsum_template.h \ - macros.h weakptr.h gpgv.h - -# Source code for the core main library -SOURCE+= pkgcache.cc version.cc depcache.cc \ - orderlist.cc tagfile.cc sourcelist.cc packagemanager.cc \ - pkgrecords.cc algorithms.cc acquire.cc\ - acquire-worker.cc acquire-method.cc init.cc clean.cc \ - srcrecords.cc cachefile.cc versionmatch.cc policy.cc \ - pkgsystem.cc indexfile.cc pkgcachegen.cc acquire-item.cc \ - indexrecords.cc vendor.cc vendorlist.cc cdrom.cc indexcopy.cc \ - aptconfiguration.cc cachefilter.cc cacheset.cc edsp.cc \ - install-progress.cc upgrade.cc update.cc -HEADERS+= algorithms.h depcache.h pkgcachegen.h cacheiterators.h \ - orderlist.h sourcelist.h packagemanager.h tagfile.h \ - init.h pkgcache.h version.h progress.h pkgrecords.h \ - acquire.h acquire-worker.h acquire-item.h acquire-method.h \ - clean.h srcrecords.h cachefile.h versionmatch.h policy.h \ - pkgsystem.h indexfile.h metaindex.h indexrecords.h vendor.h \ - vendorlist.h cdrom.h indexcopy.h aptconfiguration.h \ - cachefilter.h cacheset.h edsp.h install-progress.h \ - upgrade.h update.h - -# Source code for the debian specific components -# In theory the deb headers do not need to be exported.. -SOURCE+= deb/deblistparser.cc deb/debrecords.cc deb/dpkgpm.cc \ - deb/debsrcrecords.cc deb/debversion.cc deb/debsystem.cc \ - deb/debindexfile.cc deb/debindexfile.cc deb/debmetaindex.cc -HEADERS+= debversion.h debsrcrecords.h dpkgpm.h debrecords.h \ - deblistparser.h debsystem.h debindexfile.h debmetaindex.h - -# Source code for the APT resolver interface specific components -SOURCE+= edsp/edsplistparser.cc edsp/edspindexfile.cc edsp/edspsystem.cc -HEADERS+= edsplistparser.h edspindexfile.h edspsystem.h - -HEADERS := $(addprefix apt-pkg/,$(HEADERS)) +SOURCE = $(wildcard *.cc */*.cc) +HEADERS = $(addprefix apt-pkg/,$(notdir $(wildcard *.h */*.h))) include $(LIBRARY_H) diff --git a/apt-private/makefile b/apt-private/makefile index 09736c6d3..9a3fbdb29 100644 --- a/apt-private/makefile +++ b/apt-private/makefile @@ -8,9 +8,6 @@ HEADER_TARGETDIRS = apt-private # Bring in the default rules include ../buildlib/defaults.mak -# The library name and version (indirectly used from init.h) -include ../buildlib/libversion.mak - # The library name LIBRARY=apt-private MAJOR=0.0 @@ -18,12 +15,7 @@ MINOR=0 SLIBS=$(PTHREADLIB) -lapt-pkg CXXFLAGS += -fvisibility=hidden -fvisibility-inlines-hidden -PRIVATES=list install download output cachefile cacheset update upgrade cmndline moo search show main utils sources -SOURCE += $(foreach private, $(PRIVATES), private-$(private).cc) -HEADERS += $(foreach private, $(PRIVATES), private-$(private).h) - -SOURCE+= acqprogress.cc -HEADERS+= acqprogress.h private-cacheset.h +SOURCE = $(wildcard *.cc) +HEADERS = $(addprefix apt-private/,$(wildcard *.h)) -HEADERS := $(addprefix apt-private/,$(HEADERS)) include $(LIBRARY_H) -- cgit v1.2.3-70-g09d2 From 8056a00cd76c0f9dd6c444f23aa9998f96f805ed Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 11 Apr 2014 11:16:04 +0200 Subject: do not create an (additional) empty compressor FileFd code knows how to deal with such a compressor, so it isn't a problem, but it is absolutely not needed as we already have an (matching) identity compressor with '.' earlier in the list. Git-Dch: Ignore --- apt-pkg/aptconfiguration.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/aptconfiguration.cc b/apt-pkg/aptconfiguration.cc index 6ba047560..9982759c6 100644 --- a/apt-pkg/aptconfiguration.cc +++ b/apt-pkg/aptconfiguration.cc @@ -476,7 +476,7 @@ const Configuration::getCompressors(bool const Cached) { std::vector const comp = _config->FindVector("APT::Compressor"); for (std::vector::const_iterator c = comp.begin(); c != comp.end(); ++c) { - if (*c == "." || *c == "gzip" || *c == "bzip2" || *c == "lzma" || *c == "xz") + if (c->empty() || *c == "." || *c == "gzip" || *c == "bzip2" || *c == "lzma" || *c == "xz") continue; compressors.push_back(Compressor(c->c_str(), std::string(".").append(*c).c_str(), c->c_str(), "-9", "-d", 100)); } -- cgit v1.2.3-70-g09d2 From 21ba8115c873ae4085770e4bf0bed765e0e099a6 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 11 Apr 2014 11:18:58 +0200 Subject: don't double-count seeks in FileFd::Skip for bzip/xz FileFd::Read already deals with the increase of the skipposition so that we as the caller in FileFd::Skip really shouldn't increase it, too. --- apt-pkg/contrib/fileutl.cc | 1 - 1 file changed, 1 deletion(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index 69a675648..c51f75984 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1712,7 +1712,6 @@ bool FileFd::Skip(unsigned long long Over) { if (d != NULL && (d->pipe == true || d->InternalStream() == true)) { - d->seekpos += Over; char buffer[1024]; while (Over != 0) { -- cgit v1.2.3-70-g09d2 From 230e69d718f761a39ee3ee057938dcd0264af74f Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 11 Apr 2014 11:22:10 +0200 Subject: deal with umask only if we really need to for mkstemp As the comment actually says: open() does the umask dance by itself, so we don't need to do it for it. We have to do it after mkstemp in Atomic though, so move it into the if. Also removes the "micro-optimisation" "FilePermissions == 600" as it doesn't trigger at the moment anyway as 600 != 0600. --- apt-pkg/contrib/fileutl.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index c51f75984..c9d419fc4 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1067,13 +1067,6 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co if_FLAGGED_SET(Exclusive, O_EXCL); #undef if_FLAGGED_SET - // umask() will always set the umask and return the previous value, so - // we first set the umask and then reset it to the old value - mode_t CurrentUmask = umask(0); - umask(CurrentUmask); - // calculate the actual file permissions (just like open/creat) - mode_t FilePermissions = (AccessMode & ~CurrentUmask); - if ((Mode & Atomic) == Atomic) { char *name = strdup((FileName + ".XXXXXX").c_str()); @@ -1087,11 +1080,18 @@ bool FileFd::Open(string FileName,unsigned int const Mode,APT::Configuration::Co TemporaryFileName = string(name); free(name); - if(FilePermissions != 600 && fchmod(iFd, FilePermissions) == -1) + // umask() will always set the umask and return the previous value, so + // we first set the umask and then reset it to the old value + mode_t const CurrentUmask = umask(0); + umask(CurrentUmask); + // calculate the actual file permissions (just like open/creat) + mode_t const FilePermissions = (AccessMode & ~CurrentUmask); + + if(fchmod(iFd, FilePermissions) == -1) return FileFdErrno("fchmod", "Could not change permissions for temporary file %s", TemporaryFileName.c_str()); } else - iFd = open(FileName.c_str(), fileflags, FilePermissions); + iFd = open(FileName.c_str(), fileflags, AccessMode); this->FileName = FileName; if (iFd == -1 || OpenInternDescriptor(Mode, compressor) == false) -- cgit v1.2.3-70-g09d2 From 4cd4a2e7033a2af214be1d830b56fab719088b7a Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 11 Apr 2014 13:33:31 +0200 Subject: consider priorities only for downloadable pkgs in resolver A package which can't be downloaded anymore is very likely dropped from a release and can therefore no longer be 'standard' (or similar). We therefore do not grant points for them anymore and demote them to prio:extra instead which helps other packages breaking them away even if they have a lower priority. The testcase was initially created by Michael Vogt and just amended. --- apt-pkg/algorithms.cc | 19 +++++---- ...t-ubuntu-bug-1304403-obsolete-priority-standard | 48 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) create mode 100755 test/integration/test-ubuntu-bug-1304403-obsolete-priority-standard (limited to 'apt-pkg') diff --git a/apt-pkg/algorithms.cc b/apt-pkg/algorithms.cc index a7b676660..608ec7fce 100644 --- a/apt-pkg/algorithms.cc +++ b/apt-pkg/algorithms.cc @@ -445,19 +445,22 @@ void pkgProblemResolver::MakeScores() || (I->Flags & pkgCache::Flag::Important) == pkgCache::Flag::Important) Score += PrioEssentials; - // We transform the priority - if (Cache[I].InstVerIter(Cache)->Priority <= 5) - Score += PrioMap[Cache[I].InstVerIter(Cache)->Priority]; - + pkgCache::VerIterator const InstVer = Cache[I].InstVerIter(Cache); + // We apply priorities only to downloadable packages, all others are prio:extra + // as an obsolete prio:standard package can't be that standard anymore… + if (InstVer->Priority <= pkgCache::State::Extra && InstVer.Downloadable() == true) + Score += PrioMap[InstVer->Priority]; + else + Score += PrioMap[pkgCache::State::Extra]; + /* This helps to fix oddball problems with conflicting packages - on the same level. We enhance the score of installed packages - if those are not obsolete - */ + on the same level. We enhance the score of installed packages + if those are not obsolete */ if (I->CurrentVer != 0 && Cache[I].CandidateVer != 0 && Cache[I].CandidateVerIter(Cache).Downloadable()) Score += PrioInstalledAndNotObsolete; // propagate score points along dependencies - for (pkgCache::DepIterator D = Cache[I].InstVerIter(Cache).DependsList(); D.end() == false; ++D) + for (pkgCache::DepIterator D = InstVer.DependsList(); D.end() == false; ++D) { if (DepMap[D->Type] == 0) continue; diff --git a/test/integration/test-ubuntu-bug-1304403-obsolete-priority-standard b/test/integration/test-ubuntu-bug-1304403-obsolete-priority-standard new file mode 100755 index 000000000..2f2d384e1 --- /dev/null +++ b/test/integration/test-ubuntu-bug-1304403-obsolete-priority-standard @@ -0,0 +1,48 @@ +#!/bin/sh +set -e + +TESTDIR=$(readlink -f $(dirname $0)) +. $TESTDIR/framework + +setupenvironment +configarchitecture 'i386' + +# Regression test for LP: #1304403 +# +# The issue here is that libkadm5srv-mit8 (priority standard) is replaced +# by a new libkadm5srv-mit9 and libkbd5-7 breaks on the old -mit8 package. +# The -mit8 package is no longer downloadable (and hence not upgradeable) + +# normal upradable pkg +# (libkdb5-7 that breaks on libkadm5srv-mit8 (<< 1.11+dfsg~) +insertinstalledpackage 'upgradable' 'all' '1.0' '' 'extra' +insertpackage 'unstable' 'upgradable' 'all' '2.0' 'Breaks: not-downloadable (<< 1.1)' 'optional' + +# no longer downloadable pkg (libkadm5srv-mit8, replaced by libkadm5srv-mit9) +# but priority standard pushes it higher +insertinstalledpackage 'not-downloadable' 'all' '1.0' '' 'standard' + +setupaptarchive + +# discourage keeping obsolete high-priority packages … +testequal 'Reading package lists... +Building dependency tree... +The following packages will be REMOVED: + not-downloadable +The following packages will be upgraded: + upgradable +1 upgraded, 0 newly installed, 1 to remove and 0 not upgraded. +Remv not-downloadable [1.0] +Inst upgradable [1.0] (2.0 unstable [all]) +Conf upgradable (2.0 unstable [all])' aptget -s dist-upgrade + +# … but if it has dependencies we want to keep it as usual +for i in $(seq 1 10); do +insertinstalledpackage "depender$i" 'all' '1.0' 'Depends: not-downloadable' +done + +testequal 'Reading package lists... +Building dependency tree... +The following packages have been kept back: + upgradable +0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded.' aptget -s dist-upgrade -- cgit v1.2.3-70-g09d2 From 76bd63e2dc6ac5aaaf7f2cc98c2a83ab88ccc46c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 14 Apr 2014 17:12:09 +0200 Subject: force fancy progressbar redraw on window size change We always reacted on the size change, but the bar is only redraw if the precentage changes, which can take quiet a while in big upgrades, so with a bit of refactoring we can now call for a redraw immediate to fix this. This refactor also helps in avoiding obscure pitfalls clangs static analyser was complaining about (namely failure of ioctl resulting in garbage values in the struct). --- apt-pkg/install-progress.cc | 35 +++++++++++++++++++++++------------ apt-pkg/install-progress.h | 3 +++ 2 files changed, 26 insertions(+), 12 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/install-progress.cc b/apt-pkg/install-progress.cc index 8bb587f67..cf6c85912 100644 --- a/apt-pkg/install-progress.cc +++ b/apt-pkg/install-progress.cc @@ -256,14 +256,14 @@ PackageManagerFancy::TermSize PackageManagerFancy::GetTerminalSize() { struct winsize win; - PackageManagerFancy::TermSize s; + PackageManagerFancy::TermSize s = { 0, 0 }; // FIXME: get from "child_pty" instead? if(ioctl(STDOUT_FILENO, TIOCGWINSZ, (char *)&win) != 0) return s; if(_config->FindB("Debug::InstallProgress::Fancy", false) == true) - std::cerr << "GetTerminalSize: " << win.ws_row << std::endl; + std::cerr << "GetTerminalSize: " << win.ws_row << " x " << win.ws_col << std::endl; s.rows = win.ws_row; s.columns = win.ws_col; @@ -275,6 +275,9 @@ void PackageManagerFancy::SetupTerminalScrollArea(int nr_rows) if(_config->FindB("Debug::InstallProgress::Fancy", false) == true) std::cerr << "SetupTerminalScrollArea: " << nr_rows << std::endl; + if (unlikely(nr_rows <= 1)) + return; + // scroll down a bit to avoid visual glitch when the screen // area shrinks by one row std::cout << "\n"; @@ -296,28 +299,30 @@ void PackageManagerFancy::SetupTerminalScrollArea(int nr_rows) // setup tty size to ensure xterm/linux console are working properly too // see bug #731738 struct winsize win; - ioctl(child_pty, TIOCGWINSZ, (char *)&win); - win.ws_row = nr_rows - 1; - ioctl(child_pty, TIOCSWINSZ, (char *)&win); + if (ioctl(child_pty, TIOCGWINSZ, (char *)&win) != -1) + { + win.ws_row = nr_rows - 1; + ioctl(child_pty, TIOCSWINSZ, (char *)&win); + } } void PackageManagerFancy::HandleSIGWINCH(int) { - int nr_terminal_rows = GetTerminalSize().rows; + int const nr_terminal_rows = GetTerminalSize().rows; SetupTerminalScrollArea(nr_terminal_rows); + DrawStatusLine(); } void PackageManagerFancy::Start(int a_child_pty) { child_pty = a_child_pty; - int nr_terminal_rows = GetTerminalSize().rows; - if (nr_terminal_rows > 0) - SetupTerminalScrollArea(nr_terminal_rows); + int const nr_terminal_rows = GetTerminalSize().rows; + SetupTerminalScrollArea(nr_terminal_rows); } void PackageManagerFancy::Stop() { - int nr_terminal_rows = GetTerminalSize().rows; + int const nr_terminal_rows = GetTerminalSize().rows; if (nr_terminal_rows > 0) { SetupTerminalScrollArea(nr_terminal_rows + 1); @@ -358,7 +363,13 @@ bool PackageManagerFancy::StatusChanged(std::string PackageName, HumanReadableAction)) return false; - PackageManagerFancy::TermSize size = GetTerminalSize(); + return DrawStatusLine(); +} +bool PackageManagerFancy::DrawStatusLine() +{ + PackageManagerFancy::TermSize const size = GetTerminalSize(); + if (unlikely(size.rows < 1 || size.columns < 1)) + return false; static std::string save_cursor = "\033[s"; static std::string restore_cursor = "\033[u"; @@ -388,7 +399,7 @@ bool PackageManagerFancy::StatusChanged(std::string PackageName, { int padding = 4; float progressbar_size = size.columns - padding - progress_str.size(); - float current_percent = (float)StepsDone/(float)TotalSteps; + float current_percent = percentage / 100.0; std::cout << " " << GetTextProgressStr(current_percent, progressbar_size) << " "; diff --git a/apt-pkg/install-progress.h b/apt-pkg/install-progress.h index 112b034fb..5d1a20e9b 100644 --- a/apt-pkg/install-progress.h +++ b/apt-pkg/install-progress.h @@ -1,6 +1,8 @@ #ifndef PKGLIB_IPROGRESS_H #define PKGLIB_IPROGRESS_H +#include + #include #include #include @@ -120,6 +122,7 @@ namespace Progress { private: static void staticSIGWINCH(int); static std::vector instances; + APT_HIDDEN bool DrawStatusLine(); protected: void SetupTerminalScrollArea(int nr_rows); -- cgit v1.2.3-70-g09d2 From bb93178b8b5c2f8021977dbc34066f0d0fb8b9b9 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Tue, 15 Apr 2014 10:21:52 +0200 Subject: clear HitEof flag in FileFd::Seek fseek and co do this to their eof-flags and it is more logic this way as we will usually seek away from the end (e.g. to re-read the file). The commit also improves the testcase further and adds a test for the binary compressor codepath (as gz, bzip2 and xz are handled by libraries) via the use of 'rev' as a 'compressor'. --- apt-pkg/contrib/fileutl.cc | 7 ++- test/libapt/assert.h | 2 + test/libapt/fileutl_test.cc | 110 ++++++++++++++++++++++++++++++++++---------- test/libapt/run-tests | 2 + 4 files changed, 96 insertions(+), 25 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index c9d419fc4..de73a7fd8 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -1360,7 +1360,10 @@ bool FileFd::OpenInternDescriptor(unsigned int const Mode, APT::Configuration::C Args.push_back(a->c_str()); if (Comp == false && FileName.empty() == false) { - Args.push_back("--stdout"); + // commands not needing arguments, do not need to be told about using standard output + // in reality, only testcases with tools like cat, rev, rot13, … are able to trigger this + if (compressor.CompressArgs.empty() == false && compressor.UncompressArgs.empty() == false) + Args.push_back("--stdout"); if (TemporaryFileName.empty() == false) Args.push_back(TemporaryFileName.c_str()); else @@ -1653,6 +1656,8 @@ bool FileFd::Write(int Fd, const void *From, unsigned long long Size) /* */ bool FileFd::Seek(unsigned long long To) { + Flags &= ~HitEof; + if (d != NULL && (d->pipe == true || d->InternalStream() == true)) { // Our poor man seeking in pipes is costly, so try to avoid it diff --git a/test/libapt/assert.h b/test/libapt/assert.h index 357801592..3e45143a3 100644 --- a/test/libapt/assert.h +++ b/test/libapt/assert.h @@ -2,6 +2,7 @@ #include #include +#include #if __GNUC__ >= 4 #pragma GCC diagnostic push @@ -14,6 +15,7 @@ template < typename X, typename Y > APT_NORETURN void OutputAssertEqual(X expect, char const* compare, Y get, unsigned long const &line) { std::cerr << "Test FAILED: »" << expect << "« " << compare << " »" << get << "« at line " << line << std::endl; + _error->DumpErrors(std::cerr); std::exit(EXIT_FAILURE); } diff --git a/test/libapt/fileutl_test.cc b/test/libapt/fileutl_test.cc index 3a354814d..f3a3dd08e 100644 --- a/test/libapt/fileutl_test.cc +++ b/test/libapt/fileutl_test.cc @@ -24,7 +24,9 @@ TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission, unsigned i { FileFd f; struct stat buf; - static const char* fname = "test.txt"; + static const char* fname = "apt-filefd-test.txt"; + if (FileExists(fname) == true) + equals(unlink(fname), 0); umask(a_umask); equals(f.Open(fname, filemode, compressor), true); @@ -32,7 +34,7 @@ TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission, unsigned i equals(f.Failed(), false); equals(umask(a_umask), a_umask); - std::string test = "This is a test!"; + std::string test = "This is a test!\n"; equals(f.Write(test.c_str(), test.size()), true); equals(f.IsOpen(), true); equals(f.Failed(), false); @@ -42,12 +44,21 @@ TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission, unsigned i equals(f.Failed(), false); equals(f.Open(fname, FileFd::ReadOnly, compressor), true); - equalsNot(f.FileSize(), 0); equals(f.IsOpen(), true); equals(f.Failed(), false); + equals(f.Eof(), false); + equalsNot(f.FileSize(), 0); + equals(f.Failed(), false); + equalsNot(f.ModificationTime(), 0); + equals(f.Failed(), false); - char readback[20]; + // ensure the memory is as predictably messed up +# define APT_INIT_READBACK \ + char readback[20]; \ + memset(readback, 'D', sizeof(readback)/sizeof(readback[0])); \ + readback[19] = '\0'; { + APT_INIT_READBACK char const * const expect = "This"; equals(f.Read(readback, strlen(expect)), true); equals(f.Failed(), false); @@ -56,7 +67,8 @@ TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission, unsigned i equals(strlen(expect), f.Tell()); } { - char const * const expect = "test!"; + APT_INIT_READBACK + char const * const expect = "test!\n"; equals(f.Skip((test.size() - f.Tell()) - strlen(expect)), true); equals(f.Read(readback, strlen(expect)), true); equals(f.Failed(), false); @@ -64,22 +76,60 @@ TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission, unsigned i strequals(expect, readback); equals(test.size(), f.Tell()); } - - equals(f.Seek(0), true); - equals(f.Read(readback, 20, true), true); - equals(f.Failed(), false); - equals(f.Eof(), true); - equals(test, readback); - equals(test.size(), strlen(readback)); - equals(f.Size(), f.Tell()); - - equals(f.Seek(0), true); - f.ReadLine(readback, 20); - equals(f.Failed(), false); - equals(f.Eof(), true); - equals(test, readback); - equals(test.size(), strlen(readback)); - equals(f.Size(), f.Tell()); + { + APT_INIT_READBACK + equals(f.Seek(0), true); + equals(f.Eof(), false); + equals(f.Read(readback, 20, true), true); + equals(f.Failed(), false); + equals(f.Eof(), true); + strequals(test.c_str(), readback); + equals(f.Size(), f.Tell()); + } + { + APT_INIT_READBACK + equals(f.Seek(0), true); + equals(f.Eof(), false); + equals(f.Read(readback, test.size(), true), true); + equals(f.Failed(), false); + equals(f.Eof(), false); + strequals(test.c_str(), readback); + equals(f.Size(), f.Tell()); + } + { + APT_INIT_READBACK + equals(f.Seek(0), true); + equals(f.Eof(), false); + unsigned long long actual; + equals(f.Read(readback, 20, &actual), true); + equals(f.Failed(), false); + equals(f.Eof(), true); + equals(test.size(), actual); + strequals(test.c_str(), readback); + equals(f.Size(), f.Tell()); + } + { + APT_INIT_READBACK + equals(f.Seek(0), true); + equals(f.Eof(), false); + f.ReadLine(readback, 20); + equals(f.Failed(), false); + equals(f.Eof(), false); + equals(test, readback); + equals(f.Size(), f.Tell()); + } + { + APT_INIT_READBACK + equals(f.Seek(0), true); + equals(f.Eof(), false); + char const * const expect = "This"; + f.ReadLine(readback, strlen(expect) + 1); + equals(f.Failed(), false); + equals(f.Eof(), false); + strequals(expect, readback); + equals(strlen(expect), f.Tell()); + } +#undef APT_INIT_READBACK f.Close(); equals(f.IsOpen(), false); @@ -91,14 +141,19 @@ TestFileFd(mode_t const a_umask, mode_t const ExpectedFilePermission, unsigned i _error->Errno("stat", "failed to stat"); return false; } - unlink(fname); + equals(unlink(fname), 0); equals(buf.st_mode & 0777, ExpectedFilePermission); return true; } static bool TestFileFd(unsigned int const filemode) { - std::vector const compressors = APT::Configuration::getCompressors(); + std::vector compressors = APT::Configuration::getCompressors(); + + // testing the (un)compress via pipe, as the 'real' compressors are usually built in via libraries + compressors.push_back(APT::Configuration::Compressor("rev", ".reversed", "rev", NULL, NULL, 42)); + //compressors.push_back(APT::Configuration::Compressor("cat", ".ident", "cat", NULL, NULL, 42)); + for (std::vector::const_iterator c = compressors.begin(); c != compressors.end(); ++c) { if ((filemode & FileFd::ReadWrite) == FileFd::ReadWrite && @@ -116,8 +171,13 @@ static bool TestFileFd(unsigned int const filemode) return true; } -int main() +int main(int const argc, char const * const * const argv) { + std::string startdir; + if (argc > 1 && DirectoryExists(argv[1]) == true) { + startdir = SafeGetCWD(); + equals(chdir(argv[1]), 0); + } if (TestFileFd(FileFd::WriteOnly | FileFd::Create) == false || TestFileFd(FileFd::WriteOnly | FileFd::Create | FileFd::Empty) == false || TestFileFd(FileFd::WriteOnly | FileFd::Create | FileFd::Exclusive) == false || @@ -131,6 +191,8 @@ int main() { return 1; } + if (startdir.empty() == false) + equals(chdir(startdir.c_str()), 0); std::vector files; // normal match diff --git a/test/libapt/run-tests b/test/libapt/run-tests index 0baedcf9e..574e51e90 100755 --- a/test/libapt/run-tests +++ b/test/libapt/run-tests @@ -116,6 +116,8 @@ sysfs0 /sys0 sysfs rw,nosuid,nodev,noexec,relatime 0 0 /dev/disk/by-uuid/fadcbc52-6284-4874-aaaa-dcee1f05fe21 / ext4 rw,relatime,errors=remount-ro,data=ordered 0 0 /dev/sda1 /boot/efi vfat rw,nosuid,nodev,noexec,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=utf8,shortname=lower,quiet,utf8,errors=remount-ro,rw,nosuid,nodev,noexec,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=utf8,shortname=lower,quiet,utf8,errors=remount-ro,rw,nosuid,nodev,noexec,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=utf8,shortname=lower,quiet,utf8,errors=remount-ro,rw,nosuid,nodev,noexec,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=utf8,shortname=lower,quiet,utf8,errors=remount-ro 0 0 tmpfs /tmp tmpfs rw,nosuid,nodev,relatime 0 0' > $tmppath + elif [ $name = "FileUtl${EXT}" ]; then + tmppath="$(mktemp -d)" fi echo -n "Testing with ${NAME} " -- cgit v1.2.3-70-g09d2 From 6c50447eb443768764f83b3ba86ef32780ba6dde Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 25 Apr 2014 14:41:35 +0200 Subject: reduce delta from ubuntu --- apt-pkg/deb/dpkgpm.cc | 2 +- apt-pkg/init.cc | 1 + debian/apt.conf.autoremove | 2 ++ debian/apt.install.in | 2 +- ftparchive/override.cc | 4 ++-- 5 files changed, 7 insertions(+), 4 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index c3e7e1d1d..c52b83c6e 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -1617,7 +1617,7 @@ void pkgDPkgPM::WriteApportReport(const char *pkgpath, const char *errormsg) string::size_type pos; FILE *report; - if (_config->FindB("Dpkg::ApportFailureReport", false) == false) + if (_config->FindB("Dpkg::ApportFailureReport", true) == false) { std::clog << "configured to not write apport reports" << std::endl; return; diff --git a/apt-pkg/init.cc b/apt-pkg/init.cc index 3a35f852e..241628632 100644 --- a/apt-pkg/init.cc +++ b/apt-pkg/init.cc @@ -86,6 +86,7 @@ bool pkgInitConfig(Configuration &Cnf) Cnf.Set("Dir::Ignore-Files-Silently::", "\\.dpkg-[a-z]+$"); Cnf.Set("Dir::Ignore-Files-Silently::", "\\.save$"); Cnf.Set("Dir::Ignore-Files-Silently::", "\\.orig$"); + Cnf.Set("Dir::Ignore-Files-Silently::", "\\.distUpgrade$"); // Default cdrom mount point Cnf.CndSet("Acquire::cdrom::mount", "/media/cdrom/"); diff --git a/debian/apt.conf.autoremove b/debian/apt.conf.autoremove index cf69d56c3..fc02350ae 100644 --- a/debian/apt.conf.autoremove +++ b/debian/apt.conf.autoremove @@ -22,6 +22,8 @@ APT ".*-modules"; ".*-kernel"; "linux-backports-modules-.*"; + # tools + "linux-tools"; }; Never-MarkAuto-Sections diff --git a/debian/apt.install.in b/debian/apt.install.in index 4ffe43e3b..9c9489572 100644 --- a/debian/apt.install.in +++ b/debian/apt.install.in @@ -3,4 +3,4 @@ bin/apt-* usr/bin/ bin/methods/* usr/lib/apt/methods/ scripts/dselect/* usr/lib/dpkg/methods/apt/ usr/share/locale/*/*/apt.mo -bin/libapt-private.so.* usr/lib/@DEB_HOST_MULTIARCH@/ \ No newline at end of file +bin/libapt-private.so.* usr/lib/@DEB_HOST_MULTIARCH@/ diff --git a/ftparchive/override.cc b/ftparchive/override.cc index b4cd49b6c..8a0c5bab1 100644 --- a/ftparchive/override.cc +++ b/ftparchive/override.cc @@ -37,7 +37,7 @@ bool Override::ReadOverride(string const &File,bool const &Source) if (F == 0) return _error->Errno("fopen",_("Unable to open %s"),File.c_str()); - char Line[500]; + char Line[1000]; unsigned long long Counter = 0; while (fgets(Line,sizeof(Line),F) != 0) { @@ -141,7 +141,7 @@ bool Override::ReadExtraOverride(string const &File,bool const &/*Source*/) if (F == 0) return _error->Errno("fopen",_("Unable to open %s"),File.c_str()); - char Line[500]; + char Line[1000]; unsigned long long Counter = 0; while (fgets(Line,sizeof(Line),F) != 0) { -- cgit v1.2.3-70-g09d2 From 7187074bfc7a6932ab21c33546e71b61abe258e3 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Mon, 21 Apr 2014 11:54:34 +0200 Subject: properly undo CD-ROM mount in all error cases In bug #740673 various issues in the CD-ROM handling code were identified, while most the issues ended up being fixed in another way, the unmounting of the CD-ROM in error cases was not tackled so far. (The patch was modified by the commiter to apply) --- apt-pkg/cdrom.cc | 66 +++++++++++++++++++++++++++++++++++--------------------- apt-pkg/cdrom.h | 1 + 2 files changed, 42 insertions(+), 25 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/cdrom.cc b/apt-pkg/cdrom.cc index 2635ede76..a5ad6a9ff 100644 --- a/apt-pkg/cdrom.cc +++ b/apt-pkg/cdrom.cc @@ -563,6 +563,15 @@ bool pkgCdrom::WriteSourceList(string Name,vector &List,bool Source) return true; } /*}}}*/ +bool pkgCdrom::UnmountCDROM(std::string const &CDROM, pkgCdromStatus * const log)/*{{{*/ +{ + if (_config->FindB("APT::CDROM::NoMount",false) == true) + return true; + if (log != NULL) + log->Update(_("Unmounting CD-ROM...\n"), STEP_LAST); + return UnmountCdrom(CDROM); +} + /*}}}*/ bool pkgCdrom::MountAndIdentCDROM(Configuration &Database, std::string &CDROM, std::string &ident, pkgCdromStatus * const log, bool const interactive)/*{{{*/ { // Startup @@ -583,9 +592,7 @@ bool pkgCdrom::MountAndIdentCDROM(Configuration &Database, std::string &CDROM, s { if (interactive == true) { - if(log != NULL) - log->Update(_("Unmounting CD-ROM...\n"), STEP_LAST); - UnmountCdrom(CDROM); + UnmountCDROM(CDROM, log); if(log != NULL) { @@ -605,6 +612,9 @@ bool pkgCdrom::MountAndIdentCDROM(Configuration &Database, std::string &CDROM, s return _error->Error("Failed to mount the cdrom."); } + if (IsMounted(CDROM) == false) + return _error->Error("Failed to mount the cdrom."); + // Hash the CD to get an ID if (log != NULL) log->Update(_("Identifying... "), STEP_IDENT); @@ -614,6 +624,7 @@ bool pkgCdrom::MountAndIdentCDROM(Configuration &Database, std::string &CDROM, s ident = ""; if (log != NULL) log->Update("\n"); + UnmountCDROM(CDROM, NULL); return false; } @@ -629,8 +640,11 @@ bool pkgCdrom::MountAndIdentCDROM(Configuration &Database, std::string &CDROM, s if (FileExists(DFile) == true) { if (ReadConfigFile(Database,DFile) == false) + { + UnmountCDROM(CDROM, NULL); return _error->Error("Unable to read the cdrom database %s", DFile.c_str()); + } } return true; } @@ -651,13 +665,7 @@ bool pkgCdrom::Ident(string &ident, pkgCdromStatus *log) /*{{{*/ } // Unmount and finish - if (_config->FindB("APT::CDROM::NoMount",false) == false) - { - if (log != NULL) - log->Update(_("Unmounting CD-ROM...\n"), STEP_LAST); - UnmountCdrom(CDROM); - } - + UnmountCDROM(CDROM, log); return true; } /*}}}*/ @@ -682,11 +690,15 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ { if (log != NULL) log->Update("\n"); + UnmountCDROM(CDROM, NULL); return false; } if (chdir(StartDir.c_str()) != 0) + { + UnmountCDROM(CDROM, NULL); return _error->Errno("chdir","Unable to change to %s", StartDir.c_str()); + } if (_config->FindB("Debug::aptcdrom",false) == true) { @@ -728,8 +740,7 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ if (List.empty() == true && SourceList.empty() == true) { - if (_config->FindB("APT::CDROM::NoMount",false) == false) - UnmountCdrom(CDROM); + UnmountCDROM(CDROM, NULL); return _error->Error(_("Unable to locate any package files, perhaps this is not a Debian Disc or the wrong architecture?")); } @@ -769,14 +780,14 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ { if(log == NULL) { - if (_config->FindB("APT::CDROM::NoMount",false) == false) - UnmountCdrom(CDROM); + UnmountCDROM(CDROM, NULL); return _error->Error("No disc name found and no way to ask for it"); } while(true) { if(!log->AskCdromName(Name)) { // user canceld + UnmountCDROM(CDROM, NULL); return false; } cout << "Name: '" << Name << "'" << endl; @@ -813,7 +824,10 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ string const partialListDir = listDir + "partial/"; if (CreateAPTDirectoryIfNeeded(_config->FindDir("Dir::State"), partialListDir) == false && CreateAPTDirectoryIfNeeded(listDir, partialListDir) == false) + { + UnmountCDROM(CDROM, NULL); return _error->Errno("cdrom", _("List directory %spartial is missing."), listDir.c_str()); + } // take care of the signatures and copy them if they are ok // (we do this before PackageCopy as it modifies "List" and "SourceList") @@ -827,7 +841,10 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ if (Copy.CopyPackages(CDROM,Name,List, log) == false || SrcCopy.CopyPackages(CDROM,Name,SourceList, log) == false || TransCopy.CopyTranslations(CDROM,Name,TransList, log) == false) + { + UnmountCDROM(CDROM, NULL); return false; + } // reduce the List so that it takes less space in sources.list ReduceSourcelist(CDROM,List); @@ -837,13 +854,19 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ if (_config->FindB("APT::cdrom::NoAct",false) == false) { if (WriteDatabase(Database) == false) + { + UnmountCDROM(CDROM, NULL); return false; - + } + if(log != NULL) log->Update(_("Writing new source list\n"), STEP_WRITE); if (WriteSourceList(Name,List,false) == false || WriteSourceList(Name,SourceList,true) == false) + { + UnmountCDROM(CDROM, NULL); return false; + } } // Print the sourcelist entries @@ -855,8 +878,7 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ string::size_type Space = (*I).find(' '); if (Space == string::npos) { - if (_config->FindB("APT::CDROM::NoMount",false) == false) - UnmountCdrom(CDROM); + UnmountCDROM(CDROM, NULL); return _error->Error("Internal error"); } @@ -874,8 +896,7 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ string::size_type Space = (*I).find(' '); if (Space == string::npos) { - if (_config->FindB("APT::CDROM::NoMount",false) == false) - UnmountCdrom(CDROM); + UnmountCDROM(CDROM, NULL); return _error->Error("Internal error"); } @@ -888,12 +909,7 @@ bool pkgCdrom::Add(pkgCdromStatus *log) /*{{{*/ } // Unmount and finish - if (_config->FindB("APT::CDROM::NoMount",false) == false) { - if (log != NULL) - log->Update(_("Unmounting CD-ROM...\n"), STEP_LAST); - UnmountCdrom(CDROM); - } - + UnmountCDROM(CDROM, log); return true; } /*}}}*/ diff --git a/apt-pkg/cdrom.h b/apt-pkg/cdrom.h index 0f2c2cd02..bd0902176 100644 --- a/apt-pkg/cdrom.h +++ b/apt-pkg/cdrom.h @@ -77,6 +77,7 @@ class pkgCdrom /*{{{*/ private: APT_HIDDEN bool MountAndIdentCDROM(Configuration &Database, std::string &CDROM, std::string &ident, pkgCdromStatus * const log, bool const interactive); + APT_HIDDEN bool UnmountCDROM(std::string const &CDROM, pkgCdromStatus * const log); }; /*}}}*/ -- cgit v1.2.3-70-g09d2 From d99854cac4065bc7b337815fb2116269d58dab73 Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Mon, 21 Apr 2014 13:26:55 +0200 Subject: handle pkgnames shorter than modifiers The bugreport highlights the problem with an empty package name. We fix this by 'ignoring' these so that it behaves just like "apt-get install". The deeper problem is that modifier strings can be longer than a package name in which case the comparison doesn't make sense, so don't compare then. Was not noticed so far as all modifiers are of length 1, so the only package name shorter than this is in fact the empty package name. Closes: 744940 --- apt-pkg/cacheset.cc | 6 ++++-- test/integration/test-ubuntu-bug-365611-long-package-names | 6 +++++- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/cacheset.cc b/apt-pkg/cacheset.cc index d453a2bfb..2ed6a96da 100644 --- a/apt-pkg/cacheset.cc +++ b/apt-pkg/cacheset.cc @@ -391,6 +391,8 @@ bool VersionContainerInterface::FromModifierCommandLine(unsigned short &modID, CacheSetHelper &helper) { Version select = NEWEST; std::string str = cmdline; + if (unlikely(str.empty() == true)) + return false; bool modifierPresent = false; unsigned short fallback = modID; for (std::list::const_iterator mod = mods.begin(); @@ -400,8 +402,8 @@ bool VersionContainerInterface::FromModifierCommandLine(unsigned short &modID, size_t const alength = strlen(mod->Alias); switch(mod->Pos) { case Modifier::POSTFIX: - if (str.compare(str.length() - alength, alength, - mod->Alias, 0, alength) != 0) + if (str.length() <= alength || + str.compare(str.length() - alength, alength, mod->Alias, 0, alength) != 0) continue; str.erase(str.length() - alength); modID = mod->ID; diff --git a/test/integration/test-ubuntu-bug-365611-long-package-names b/test/integration/test-ubuntu-bug-365611-long-package-names index 894c8dc97..f22986e21 100755 --- a/test/integration/test-ubuntu-bug-365611-long-package-names +++ b/test/integration/test-ubuntu-bug-365611-long-package-names @@ -4,8 +4,12 @@ set -e TESTDIR=$(readlink -f $(dirname $0)) . $TESTDIR/framework setupenvironment -configarchitecture "i386" +configarchitecture 'i386' setupaptarchive aptget install $(for i in $(seq 0 1000); do echo -n 'a'; done) 2> longpackagename.log > /dev/null || true testfileequal 'longpackagename.log' "E: Unable to locate package $(for i in $(seq 0 1000); do echo -n 'a'; done)" + +# … and the opposite of long: +aptget install "" -s >longpackagename.log 2>&1 || true +testfileequal 'longpackagename.log' "$(aptget install -s)" -- cgit v1.2.3-70-g09d2 From 05eab8afb692823f86c53c4c2ced783a7c185cf9 Mon Sep 17 00:00:00 2001 From: Adam Conrad Date: Sat, 26 Apr 2014 10:24:40 +0200 Subject: fix FileFd::Size bitswap on big-endian architectures gzip only gives us 32bit of size, storing it in a 64bit container and doing a 32bit flip on it has therefore unintended results. So we just go with a exact size container and let the flipping be handled by eglibc provided le32toh removing our #ifdef machinery. Closes: 745866 --- apt-pkg/contrib/fileutl.cc | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc index de73a7fd8..b77c7ff7f 100644 --- a/apt-pkg/contrib/fileutl.cc +++ b/apt-pkg/contrib/fileutl.cc @@ -58,13 +58,10 @@ #include #endif #ifdef HAVE_LZMA - #include #include #endif - -#ifdef WORDS_BIGENDIAN -#include -#endif +#include +#include #include /*}}}*/ @@ -1880,19 +1877,13 @@ unsigned long long FileFd::Size() FileFdErrno("lseek","Unable to seek to end of gzipped file"); return 0; } - size = 0; + uint32_t size = 0; if (read(iFd, &size, 4) != 4) { FileFdErrno("read","Unable to read original size of gzipped file"); return 0; } - -#ifdef WORDS_BIGENDIAN - uint32_t tmp_size = size; - uint8_t const * const p = (uint8_t const * const) &tmp_size; - tmp_size = (p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]; - size = tmp_size; -#endif + size = le32toh(size); if (lseek(iFd, oldPos, SEEK_SET) < 0) { -- cgit v1.2.3-70-g09d2 From a11f6c973bc0dc226d8953e3edb6333d526c3143 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 30 Apr 2014 17:04:29 +0200 Subject: Only do openpty() if both stdin/stdout are terminals Closes: 746434 --- apt-pkg/deb/dpkgpm.cc | 11 ++++++----- test/integration/test-failing-maintainer-scripts | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) (limited to 'apt-pkg') diff --git a/apt-pkg/deb/dpkgpm.cc b/apt-pkg/deb/dpkgpm.cc index c52b83c6e..e410594df 100644 --- a/apt-pkg/deb/dpkgpm.cc +++ b/apt-pkg/deb/dpkgpm.cc @@ -1053,14 +1053,15 @@ void pkgDPkgPM::StartPtyMagic() } // setup the pty and stuff - struct winsize win; + struct winsize win; - // if tcgetattr does not return zero there was a error - // and we do not do any pty magic + // if tcgetattr for both stdin/stdout returns 0 (no error) + // we do the pty magic _error->PushToStack(); - if (tcgetattr(STDOUT_FILENO, &d->tt) == 0) + if (tcgetattr(STDIN_FILENO, &d->tt) == 0 && + tcgetattr(STDOUT_FILENO, &d->tt) == 0) { - if (ioctl(1, TIOCGWINSZ, (char *)&win) < 0) + if (ioctl(STDOUT_FILENO, TIOCGWINSZ, (char *)&win) < 0) { _error->Errno("ioctl", _("ioctl(TIOCGWINSZ) failed")); } else if (openpty(&d->master, &d->slave, NULL, &d->tt, &win) < 0) diff --git a/test/integration/test-failing-maintainer-scripts b/test/integration/test-failing-maintainer-scripts index cb82ebc7a..3dd7d643e 100755 --- a/test/integration/test-failing-maintainer-scripts +++ b/test/integration/test-failing-maintainer-scripts @@ -86,7 +86,7 @@ testmyfailure() { testfailure "$@" -o APT::Status-Fd=3 msgtest 'Test for failure message of maintainerscript in' 'console log' local TEST='rootdir/tmp/testfailure.output' - if grep -q 'exit status 29$' "$TEST"; then + if grep -q 'exit status 29' "$TEST"; then msgpass else cat $TEST -- cgit v1.2.3-70-g09d2