From eab9e87bd3e73cca926f0acf7554d26c08c865f2 Mon Sep 17 00:00:00 2001 From: Pavla Kratochvilova Date: Tue, 27 Jul 2021 15:33:56 +0200 Subject: [PATCH] Fix some covscan warnings Resolves: rhbz#1938761 --- 0001-Fix-some-covscan-warnings.patch | 202 +++++++++++++++++++++++++++ libdnf.spec | 6 +- 2 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 0001-Fix-some-covscan-warnings.patch diff --git a/0001-Fix-some-covscan-warnings.patch b/0001-Fix-some-covscan-warnings.patch new file mode 100644 index 0000000..c3dee0b --- /dev/null +++ b/0001-Fix-some-covscan-warnings.patch @@ -0,0 +1,202 @@ +From 153a8ae657a0d2a03f43db076bdadfb4e7dff41e Mon Sep 17 00:00:00 2001 +From: Aleš Matěj +Date: Mon, 21 Jun 2021 12:20:54 +0200 +Subject: [PATCH] Fix some covscan warnings + +- missing return value checks +- remove useless/unused code +- set fp to NULL to prevent double free false positive +- checksum function in utils.cpp doesn't have a declaration and + is used only in utils.cpp -> make it static + +Some of these were also compiler warnings +--- + libdnf/dnf-repo.cpp | 15 ++++++++++----- + libdnf/dnf-transaction.cpp | 2 -- + libdnf/module/ModulePackageContainer.cpp | 2 -- + libdnf/repo/Repo.cpp | 28 +++++++++++++++++++++++----- + libdnf/utils/filesystem.cpp | 9 ++++++++- + libdnf/utils/utils.cpp | 2 +- + tests/hawkey/test_iutil.cpp | 2 ++ + 7 files changed, 44 insertions(+), 16 deletions(-) + +diff --git a/libdnf/dnf-repo.cpp b/libdnf/dnf-repo.cpp +index 710045f..cc6093e 100644 +--- a/libdnf/dnf-repo.cpp ++++ b/libdnf/dnf-repo.cpp +@@ -1966,9 +1966,12 @@ out: + g_free(updatedata.last_mirror_failure_message); + g_free(updatedata.last_mirror_url); + dnf_state_release_locks(state); +- lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSCB, NULL); +- lr_handle_setopt(priv->repo_handle, NULL, LRO_HMFCB, NULL); +- lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSDATA, 0xdeadbeef); ++ if (!lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSCB, NULL)) ++ g_debug("Failed to reset LRO_PROGRESSCB to NULL"); ++ if (!lr_handle_setopt(priv->repo_handle, NULL, LRO_HMFCB, NULL)) ++ g_debug("Failed to reset LRO_HMFCB to NULL"); ++ if (!lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSDATA, 0xdeadbeef)) ++ g_debug("Failed to set LRO_PROGRESSDATA to 0xdeadbeef"); + return ret; + } CATCH_TO_GERROR(FALSE) + +@@ -2296,8 +2299,10 @@ dnf_repo_download_packages(DnfRepo *repo, + + ret = TRUE; + out: +- lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSCB, NULL); +- lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSDATA, 0xdeadbeef); ++ if (!lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSCB, NULL)) ++ g_debug("Failed to reset LRO_PROGRESSCB to NULL"); ++ if (!lr_handle_setopt(priv->repo_handle, NULL, LRO_PROGRESSDATA, 0xdeadbeef)) ++ g_debug("Failed to set LRO_PROGRESSDATA to 0xdeadbeef"); + g_free(global_data.last_mirror_failure_message); + g_free(global_data.last_mirror_url); + g_slist_free_full(package_targets, (GDestroyNotify)lr_packagetarget_free); +diff --git a/libdnf/dnf-transaction.cpp b/libdnf/dnf-transaction.cpp +index 7c9a3a9..e096658 100644 +--- a/libdnf/dnf-transaction.cpp ++++ b/libdnf/dnf-transaction.cpp +@@ -1523,8 +1523,6 @@ dnf_transaction_commit(DnfTransaction *transaction, HyGoal goal, DnfState *state + + /* this section done */ + ret = dnf_state_done(state, error); +- if (!ret) +- goto out; + out: + dnf_transaction_reset(transaction); + dnf_state_release_locks(state); +diff --git a/libdnf/module/ModulePackageContainer.cpp b/libdnf/module/ModulePackageContainer.cpp +index a20f8df..dc551a9 100644 +--- a/libdnf/module/ModulePackageContainer.cpp ++++ b/libdnf/module/ModulePackageContainer.cpp +@@ -1753,8 +1753,6 @@ void ModulePackageContainer::updateFailSafeData() + if (pImpl->activatedModules) { + std::vector latest = pImpl->getLatestActiveEnabledModules(); + +- auto begin = fileNames.begin(); +- auto end = fileNames.end(); + if (g_mkdir_with_parents(pImpl->persistDir.c_str(), 0755) == -1) { + const char * errTxt = strerror(errno); + auto logger(Log::getLogger()); +diff --git a/libdnf/repo/Repo.cpp b/libdnf/repo/Repo.cpp +index e3a574f..e1d5529 100644 +--- a/libdnf/repo/Repo.cpp ++++ b/libdnf/repo/Repo.cpp +@@ -704,7 +704,11 @@ static std::vector rawkey2infos(int fd) { + + // set GPG home dir + char tmpdir[] = "/tmp/tmpdir.XXXXXX"; +- mkdtemp(tmpdir); ++ if (!mkdtemp(tmpdir)) { ++ const char * errTxt = strerror(errno); ++ throw RepoError(tfm::format(_("Cannot create repo temporary directory \"%s\": %s"), ++ tmpdir, errTxt)); ++ } + Finalizer tmpDirRemover([&tmpdir](){ + dnf_remove_recursive(tmpdir, NULL); + }); +@@ -894,8 +898,14 @@ void Repo::Impl::importRepoKeys() + } + + struct stat sb; +- if (stat(gpgDir.c_str(), &sb) != 0 || !S_ISDIR(sb.st_mode)) +- mkdir(gpgDir.c_str(), 0777); ++ if (stat(gpgDir.c_str(), &sb) != 0 || !S_ISDIR(sb.st_mode)) { ++ int res = mkdir(gpgDir.c_str(), 0777); ++ if (res != 0 && errno != EEXIST) { ++ auto msg = tfm::format(_("Failed to create directory \"%s\": %d - %s"), ++ gpgDir, errno, strerror(errno)); ++ throw RepoError(msg); ++ } ++ } + + gpgme_ctx_t ctx; + gpgme_new(&ctx); +@@ -1147,7 +1157,11 @@ bool Repo::Impl::isMetalinkInSync() + { + auto logger(Log::getLogger()); + char tmpdir[] = "/tmp/tmpdir.XXXXXX"; +- mkdtemp(tmpdir); ++ if (!mkdtemp(tmpdir)) { ++ const char * errTxt = strerror(errno); ++ throw RepoError(tfm::format(_("Cannot create repo temporary directory \"%s\": %s"), ++ tmpdir, errTxt)); ++ } + Finalizer tmpDirRemover([&tmpdir](){ + dnf_remove_recursive(tmpdir, NULL); + }); +@@ -1217,7 +1231,11 @@ bool Repo::Impl::isRepomdInSync() + auto logger(Log::getLogger()); + LrYumRepo *yum_repo; + char tmpdir[] = "/tmp/tmpdir.XXXXXX"; +- mkdtemp(tmpdir); ++ if (!mkdtemp(tmpdir)) { ++ const char * errTxt = strerror(errno); ++ throw RepoError(tfm::format(_("Cannot create repo temporary directory \"%s\": %s"), ++ tmpdir, errTxt)); ++ } + Finalizer tmpDirRemover([&tmpdir](){ + dnf_remove_recursive(tmpdir, NULL); + }); +diff --git a/libdnf/utils/filesystem.cpp b/libdnf/utils/filesystem.cpp +index c3abd3c..89a9478 100644 +--- a/libdnf/utils/filesystem.cpp ++++ b/libdnf/utils/filesystem.cpp +@@ -24,6 +24,8 @@ + #include + #include + ++#include "bgettext/bgettext-lib.h" ++#include "tinyformat/tinyformat.hpp" + #include "filesystem.hpp" + #include "../error.hpp" + +@@ -72,7 +74,12 @@ makeDirPath(std::string filePath) + previous = position; + // create directory if necessary + if (!pathExists(directory.c_str())) { +- mkdir(directory.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); ++ int res = mkdir(directory.c_str(), S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH); ++ if (res != 0 && errno != EEXIST) { ++ auto msg = tfm::format(_("Failed to create directory \"%s\": %d - %s"), ++ directory, errno, strerror(errno)); ++ throw Error(msg); ++ } + } + } + } +diff --git a/libdnf/utils/utils.cpp b/libdnf/utils/utils.cpp +index 450718d..15f5275 100644 +--- a/libdnf/utils/utils.cpp ++++ b/libdnf/utils/utils.cpp +@@ -301,7 +301,7 @@ void decompress(const char * inPath, const char * outPath, mode_t outMode, const + fclose(inFile); + } + +-void checksum(const char * type, const char * inPath, const char * checksum_valid, bool * valid_out, gchar ** calculated_out) ++static void checksum(const char * type, const char * inPath, const char * checksum_valid, bool * valid_out, gchar ** calculated_out) + { + GError * errP{nullptr}; + gboolean valid; +diff --git a/tests/hawkey/test_iutil.cpp b/tests/hawkey/test_iutil.cpp +index 8d00cc9..6dafbdc 100644 +--- a/tests/hawkey/test_iutil.cpp ++++ b/tests/hawkey/test_iutil.cpp +@@ -79,11 +79,13 @@ START_TEST(test_checksum) + /* the taken checksum are not zeros anymore */ + fail_if(checksum_cmp(cs1, cs2) == 0); + fail_if(checksum_cmp(cs1_sum, cs2_sum) == 0); ++ fp = NULL; + + /* append something */ + fail_if((fp = fopen(new_file, "a")) == NULL); + fail_unless(fwrite("X", 1, 1, fp) == 1); + fclose(fp); ++ fp = NULL; + + /* take the second checksums */ + fail_if((fp = fopen(new_file, "r")) == NULL); +-- +libgit2 1.0.1 + diff --git a/libdnf.spec b/libdnf.spec index 76954ba..e392ccf 100644 --- a/libdnf.spec +++ b/libdnf.spec @@ -56,11 +56,12 @@ Name: libdnf Version: %{libdnf_major_version}.%{libdnf_minor_version}.%{libdnf_micro_version} -Release: 2%{?dist} +Release: 3%{?dist} Summary: Library providing simplified C and Python API to libsolv License: LGPLv2+ URL: https://github.com/rpm-software-management/libdnf Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz +Patch1: 0001-Fix-some-covscan-warnings.patch BuildRequires: cmake BuildRequires: gcc @@ -305,6 +306,9 @@ popd %endif %changelog +* Tue Jul 27 2021 Pavla Kratochvilova - 0.63.0-3 +- Fix some covscan warnings + * Wed Jun 16 2021 Mohan Boddu - 0.63.0-2 - Rebuilt for RHEL 9 BETA for openssl 3.0 Related: rhbz#1971065