From 472376be6818b5ea43250abcbecfcab53b4a729a Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 1 Apr 2022 13:45:09 +0200 Subject: Use pkgTagSection::Key in more places in src:apt The speed critical paths were converted earlier, but the remaining could benefit a tiny bit from this as well especially as we have the facility now available and can therefore brush up the code in various places in the process as well. Also takes the time to add the hidden Exists method advertised in the headers, but previously not implemented. --- apt-private/private-show.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'apt-private') diff --git a/apt-private/private-show.cc b/apt-private/private-show.cc index b56b87d67..46c4c35e7 100644 --- a/apt-private/private-show.cc +++ b/apt-private/private-show.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -233,13 +234,15 @@ static bool DisplayRecordV2(pkgCacheFile &CacheFile, pkgRecords &Recs, /*{{{*/ // make size nice std::string installed_size; - if (Tags.FindULL("Installed-Size") > 0) - strprintf(installed_size, "%sB", SizeToStr(Tags.FindULL("Installed-Size") * 1024).c_str()); + auto const installed_size_field = Tags.FindULL(pkgTagSection::Key::Installed_Size); + if (installed_size_field > 0) + installed_size = SizeToStr(installed_size_field * 1024).append("B"); else installed_size = _("unknown"); std::string package_size; - if (Tags.FindULL("Size") > 0) - strprintf(package_size, "%sB", SizeToStr(Tags.FindULL("Size")).c_str()); + auto const package_size_field = Tags.FindULL(pkgTagSection::Key::Size); + if (package_size_field > 0) + package_size = SizeToStr(package_size_field).append("B"); else package_size = _("unknown"); -- cgit v1.2.3-70-g09d2 From 05fae6fae95d8ef6690f3d56863e3bb6a44d424c Mon Sep 17 00:00:00 2001 From: David Kalnischkies Date: Fri, 1 Apr 2022 11:37:26 +0200 Subject: Parse Checksum fields via pkgTagSection::Key, too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We abstract hashes a fair bit to be able to add new ones eventually, which lead us to building the field names on the fly. We can do better through by keeping a central place for these names, too, which even helps in reducing code as we don't need the MD5 → Files dance anymore. --- apt-pkg/contrib/hashes.cc | 11 +++++++++++ apt-pkg/contrib/hashes.h | 14 ++++++++++++++ apt-pkg/deb/debmetaindex.cc | 7 +++---- apt-pkg/deb/debsrcrecords.cc | 21 +++++++-------------- apt-pkg/indexcopy.cc | 18 +++++------------- apt-private/private-show.cc | 6 ++---- ftparchive/writer.cc | 10 +++++----- .../test-ubuntu-bug-1098738-apt-get-source-md5sum | 2 +- 8 files changed, 48 insertions(+), 41 deletions(-) (limited to 'apt-private') diff --git a/apt-pkg/contrib/hashes.cc b/apt-pkg/contrib/hashes.cc index 267e2679a..313b1d37d 100644 --- a/apt-pkg/contrib/hashes.cc +++ b/apt-pkg/contrib/hashes.cc @@ -17,6 +17,8 @@ #include #include #include +#include +#include #include #include @@ -45,6 +47,15 @@ const char * HashString::_SupportedHashes[] = { "SHA512", "SHA256", "SHA1", "MD5Sum", "Checksum-FileSize", NULL }; +std::vector HashString::SupportedHashesInfo() +{ + return {{ + { "SHA512", pkgTagSection::Key::SHA512,"Checksums-Sha512", pkgTagSection::Key::Checksums_Sha512}, + { "SHA256", pkgTagSection::Key::SHA256, "Checksums-Sha256", pkgTagSection::Key::Checksums_Sha256}, + { "SHA1", pkgTagSection::Key::SHA1, "Checksums-Sha1", pkgTagSection::Key::Checksums_Sha1 }, + { "MD5Sum", pkgTagSection::Key::MD5sum, "Files", pkgTagSection::Key::Files }, + }}; +} HashString::HashString() { diff --git a/apt-pkg/contrib/hashes.h b/apt-pkg/contrib/hashes.h index 422c1e023..e259b4e28 100644 --- a/apt-pkg/contrib/hashes.h +++ b/apt-pkg/contrib/hashes.h @@ -14,6 +14,11 @@ #include +#ifdef APT_COMPILING_APT +#include +#include +#endif + #include #include #include @@ -59,6 +64,15 @@ class APT_PUBLIC HashString // return the list of hashes we support static APT_PURE const char** SupportedHashes(); +#ifdef APT_COMPILING_APT + struct APT_HIDDEN HashSupportInfo { + APT::StringView name; + pkgTagSection::Key namekey; + APT::StringView chksumsname; + pkgTagSection::Key chksumskey; + }; + APT_HIDDEN static std::vector SupportedHashesInfo(); +#endif }; class APT_PUBLIC HashStringList diff --git a/apt-pkg/deb/debmetaindex.cc b/apt-pkg/deb/debmetaindex.cc index 32f1fa8db..c6005f1c8 100644 --- a/apt-pkg/deb/debmetaindex.cc +++ b/apt-pkg/deb/debmetaindex.cc @@ -515,10 +515,9 @@ bool debReleaseIndex::Load(std::string const &Filename, std::string * const Erro bool FoundHashSum = false; bool FoundStrongHashSum = false; - auto const SupportedHashes = HashString::SupportedHashes(); - for (int i=0; SupportedHashes[i] != NULL; i++) + for (auto const hashinfo : HashString::SupportedHashesInfo()) { - if (!Section.Find(SupportedHashes[i], Start, End)) + if (not Section.Find(hashinfo.namekey, Start, End)) continue; std::string Name; @@ -529,7 +528,7 @@ bool debReleaseIndex::Load(std::string const &Filename, std::string * const Erro if (!parseSumData(Start, End, Name, Hash, Size)) return false; - HashString const hs(SupportedHashes[i], Hash); + HashString const hs(hashinfo.name.to_string(), Hash); if (Entries.find(Name) == Entries.end()) { metaIndex::checkSum *Sum = new metaIndex::checkSum; diff --git a/apt-pkg/deb/debsrcrecords.cc b/apt-pkg/deb/debsrcrecords.cc index 1fd0fdc12..311bacfdd 100644 --- a/apt-pkg/deb/debsrcrecords.cc +++ b/apt-pkg/deb/debsrcrecords.cc @@ -172,26 +172,19 @@ bool debSrcRecordParser::Files(std::vector &List) std::vector const compExts = APT::Configuration::getCompressorExtensions(); auto const &posix = std::locale::classic(); - for (char const * const * type = HashString::SupportedHashes(); *type != NULL; ++type) + for (auto const hashinfo : HashString::SupportedHashesInfo()) { - // derive field from checksum type - std::string checksumField("Checksums-"); - if (strcmp(*type, "MD5Sum") == 0) - checksumField = "Files"; // historic name for MD5 checksums - else - checksumField.append(*type); - - string const Files = Sect.FindS(checksumField.c_str()); + auto const Files = Sect.Find(hashinfo.chksumskey); if (Files.empty() == true) continue; - std::istringstream ss(Files); + std::istringstream ss(Files.to_string()); ss.imbue(posix); while (ss.good()) { std::string hash, path; unsigned long long size; - if (iIndex == nullptr && checksumField == "Files") + if (iIndex == nullptr && hashinfo.chksumskey == pkgTagSection::Key::Files) { std::string ignore; ss >> hash >> size >> ignore >> ignore >> path; @@ -200,9 +193,9 @@ bool debSrcRecordParser::Files(std::vector &List) ss >> hash >> size >> path; if (ss.fail() || hash.empty() || path.empty()) - return _error->Error("Error parsing file record in %s of source package %s", checksumField.c_str(), Package().c_str()); + return _error->Error("Error parsing file record in %s of source package %s", hashinfo.chksumsname.to_string().c_str(), Package().c_str()); - HashString const hashString(*type, hash); + HashString const hashString(hashinfo.name.to_string(), hash); if (Base.empty() == false) path = Base + path; @@ -217,7 +210,7 @@ bool debSrcRecordParser::Files(std::vector &List) { // an error here indicates that we have two different hashes for the same file if (file->Hashes.push_back(hashString) == false) - return _error->Error("Error parsing checksum in %s of source package %s", checksumField.c_str(), Package().c_str()); + return _error->Error("Error parsing checksum in %s of source package %s", hashinfo.chksumsname.to_string().c_str(), Package().c_str()); continue; } diff --git a/apt-pkg/indexcopy.cc b/apt-pkg/indexcopy.cc index 82baba337..0d569265d 100644 --- a/apt-pkg/indexcopy.cc +++ b/apt-pkg/indexcopy.cc @@ -437,22 +437,14 @@ bool PackageCopy::RewriteEntry(FileFd &Target,string const &File) /* */ bool SourceCopy::GetFile(string &File,unsigned long long &Size) { - string Files; - - for (char const *const *type = HashString::SupportedHashes(); *type != NULL; ++type) + std::string Files; + for (auto hashinfo : HashString::SupportedHashesInfo()) { - // derive field from checksum type - std::string checksumField("Checksums-"); - if (strcmp(*type, "MD5Sum") == 0) - checksumField = "Files"; // historic name for MD5 checksums - else - checksumField.append(*type); - - Files = Section->FindS(checksumField); - if (Files.empty() == false) + Files = Section->Find(hashinfo.chksumskey).to_string(); + if (not Files.empty()) break; } - if (Files.empty() == true) + if (Files.empty()) return false; // Read the first file triplet diff --git a/apt-private/private-show.cc b/apt-private/private-show.cc index 46c4c35e7..08a4fe6df 100644 --- a/apt-private/private-show.cc +++ b/apt-private/private-show.cc @@ -260,10 +260,8 @@ static bool DisplayRecordV2(pkgCacheFile &CacheFile, pkgRecords &Recs, /*{{{*/ // delete, apt-cache show has this info and most users do not care if (not _config->FindB("APT::Cache::ShowFull", false)) { - RW.push_back(pkgTagSection::Tag::Remove("MD5sum")); - RW.push_back(pkgTagSection::Tag::Remove("SHA1")); - RW.push_back(pkgTagSection::Tag::Remove("SHA256")); - RW.push_back(pkgTagSection::Tag::Remove("SHA512")); + for (char const * const * type = HashString::SupportedHashes(); *type != nullptr; ++type) + RW.push_back(pkgTagSection::Tag::Remove(*type)); RW.push_back(pkgTagSection::Tag::Remove("Filename")); RW.push_back(pkgTagSection::Tag::Remove("Multi-Arch")); RW.push_back(pkgTagSection::Tag::Remove("Conffiles")); diff --git a/ftparchive/writer.cc b/ftparchive/writer.cc index c786be6ac..ae427b120 100644 --- a/ftparchive/writer.cc +++ b/ftparchive/writer.cc @@ -771,21 +771,21 @@ bool SourcesWriter::DoPackage(string FileName) { if (hs->HashType() == "MD5Sum" || hs->HashType() == "Checksum-FileSize") continue; - char const * fieldname; + pkgTagSection::Key fieldkey; std::string * out; if (hs->HashType() == "SHA1") { - fieldname = "Checksums-Sha1"; + fieldkey = pkgTagSection::Key::Checksums_Sha1; out = &ChecksumsSha1; } else if (hs->HashType() == "SHA256") { - fieldname = "Checksums-Sha256"; + fieldkey = pkgTagSection::Key::Checksums_Sha256; out = &ChecksumsSha256; } else if (hs->HashType() == "SHA512") { - fieldname = "Checksums-Sha512"; + fieldkey = pkgTagSection::Key::Checksums_Sha512; out = &ChecksumsSha512; } else @@ -793,7 +793,7 @@ bool SourcesWriter::DoPackage(string FileName) _error->Warning("Ignoring unknown Checksumtype %s in SourcesWriter::DoPackages", hs->HashType().c_str()); continue; } - if (Tags.Exists(fieldname) == true) + if (Tags.Exists(fieldkey)) continue; std::ostringstream streamout; streamout << "\n " << hs->HashValue() << " " << std::to_string(Db.GetFileSize()) << " " << ParseJnk; diff --git a/test/integration/test-ubuntu-bug-1098738-apt-get-source-md5sum b/test/integration/test-ubuntu-bug-1098738-apt-get-source-md5sum index f0a8835a2..6a66094de 100755 --- a/test/integration/test-ubuntu-bug-1098738-apt-get-source-md5sum +++ b/test/integration/test-ubuntu-bug-1098738-apt-get-source-md5sum @@ -441,4 +441,4 @@ testnohash pkg-md5-agree testfailureequal 'Reading package lists... E: Error parsing checksum in Files of source package pkg-md5-disagree' aptget source -d pkg-md5-disagree testfailureequal 'Reading package lists... -E: Error parsing checksum in Checksums-SHA256 of source package pkg-sha256-disagree' aptget source -d pkg-sha256-disagree +E: Error parsing checksum in Checksums-Sha256 of source package pkg-sha256-disagree' aptget source -d pkg-sha256-disagree -- cgit v1.2.3-70-g09d2