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