From fd9d39cf19084dd4244943a3b27ee0a33805f76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 21 Jun 2021 15:04:51 +0200 Subject: [PATCH 1/2] Fix important covscan warnings - covscan thinks we are leaking storage from `g_strndup` so be more explicit that `g_strstrip` is in place. --- librepo/downloader.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/librepo/downloader.c b/librepo/downloader.c index c5278fbc..f4e8ba2a 100644 --- a/librepo/downloader.c +++ b/librepo/downloader.c @@ -494,7 +494,10 @@ lr_headercb(void *ptr, size_t size, size_t nmemb, void *userdata) return lr_zckheadercb(ptr, size, nmemb, userdata); #endif /* WITH_ZCHUNK */ - char *header = g_strstrip(g_strndup(ptr, size*nmemb)); + char *header = g_strndup(ptr, size*nmemb); + // strips in place + g_strstrip(header); + gint64 expected = lrtarget->target->expectedsize; if (state == LR_HCS_DEFAULT) { From 5a9e9e99c64e2dc1d181fa14cf348c7f51611a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 21 Jun 2021 15:05:13 +0200 Subject: [PATCH 2/2] Fix covscan warnings - remove unused code - missing return value checks - use mkstemp just like other tests --- librepo/handle.c | 12 +++++++++--- tests/test_downloader.c | 40 ++++++++++++++++++++-------------------- tests/test_repoconf.c | 22 +++------------------- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/librepo/handle.c b/librepo/handle.c index 8ac7234c..2c23ed09 100644 --- a/librepo/handle.c +++ b/librepo/handle.c @@ -628,7 +628,7 @@ lr_handle_setopt(LrHandle *handle, "Value of LRO_LOWSPEEDTIME is too low."); ret = FALSE; } else { - curl_easy_setopt(c_h, CURLOPT_LOW_SPEED_TIME, val_long); + c_rc = curl_easy_setopt(c_h, CURLOPT_LOW_SPEED_TIME, val_long); handle->lowspeedtime = val_long; } @@ -648,7 +648,7 @@ lr_handle_setopt(LrHandle *handle, val_long, handle->maxspeed); ret = FALSE; } else { - curl_easy_setopt(c_h, CURLOPT_LOW_SPEED_LIMIT, val_long); + c_rc = curl_easy_setopt(c_h, CURLOPT_LOW_SPEED_LIMIT, val_long); handle->lowspeedlimit = val_long; } @@ -885,7 +885,13 @@ lr_yum_download_url_retry(int attempts, LrHandle *lr_handle, const char *url, g_debug("%s: Attempt #%d to download %s failed: %s", __func__, i, url, (*err)->message); - ftruncate(fd, 0); + + if (ftruncate(fd, 0) < 0) { + g_set_error(err, LR_DOWNLOADER_ERROR, LRE_IO, + "ftruncate() failed: %s", g_strerror(errno)); + return FALSE; + } + g_clear_error (err); } } diff --git a/tests/test_downloader.c b/tests/test_downloader.c index 3a1c60f4..e6155105 100644 --- a/tests/test_downloader.c +++ b/tests/test_downloader.c @@ -46,7 +46,7 @@ START_TEST(test_downloader_single_file) fail_if(handle == NULL); char *urls[] = {"http://www.google.com", NULL}; - lr_handle_setopt(handle, NULL, LRO_URLS, urls); + fail_if(!lr_handle_setopt(handle, NULL, LRO_URLS, urls)); lr_handle_prepare_internal_mirrorlist(handle, FALSE, &tmp_err); fail_if(tmp_err); @@ -55,8 +55,7 @@ START_TEST(test_downloader_single_file) tmpfn1 = lr_pathconcat(test_globals.tmpdir, "single_file_XXXXXX", NULL); - mktemp(tmpfn1); - fd1 = open(tmpfn1, O_RDWR|O_CREAT|O_TRUNC, 0666); + fd1 = mkstemp(tmpfn1); lr_free(tmpfn1); fail_if(fd1 < 0); @@ -86,6 +85,7 @@ START_TEST(test_downloader_single_file) } g_slist_free_full(list, (GDestroyNotify) lr_downloadtarget_free); + close(fd1); } END_TEST @@ -102,8 +102,7 @@ START_TEST(test_downloader_single_file_2) tmpfn1 = lr_pathconcat(test_globals.tmpdir, "single_file_2_XXXXXX", NULL); - mktemp(tmpfn1); - fd1 = open(tmpfn1, O_RDWR|O_CREAT|O_TRUNC, 0666); + fd1 = mkstemp(tmpfn1); lr_free(tmpfn1); fail_if(fd1 < 0); @@ -131,6 +130,7 @@ START_TEST(test_downloader_single_file_2) } g_slist_free_full(list, (GDestroyNotify) lr_downloadtarget_free); + close(fd1); } END_TEST @@ -151,7 +151,7 @@ START_TEST(test_downloader_two_files) fail_if(handle == NULL); char *urls[] = {"http://www.google.com", NULL}; - lr_handle_setopt(handle, NULL, LRO_URLS, urls); + fail_if(!lr_handle_setopt(handle, NULL, LRO_URLS, urls)); lr_handle_prepare_internal_mirrorlist(handle, FALSE, &tmp_err); fail_if(tmp_err); @@ -160,10 +160,8 @@ START_TEST(test_downloader_two_files) tmpfn1 = lr_pathconcat(test_globals.tmpdir, "single_file_1_XXXXXX", NULL); tmpfn2 = lr_pathconcat(test_globals.tmpdir, "single_file_2_XXXXXX", NULL); - mktemp(tmpfn1); - mktemp(tmpfn2); - fd1 = open(tmpfn1, O_RDWR|O_CREAT|O_TRUNC, 0666); - fd2 = open(tmpfn2, O_RDWR|O_CREAT|O_TRUNC, 0666); + fd1 = mkstemp(tmpfn1); + fd2 = mkstemp(tmpfn2); lr_free(tmpfn1); lr_free(tmpfn2); fail_if(fd1 < 0); @@ -200,6 +198,8 @@ START_TEST(test_downloader_two_files) } g_slist_free_full(list, (GDestroyNotify) lr_downloadtarget_free); + close(fd1); + close(fd2); } END_TEST @@ -220,7 +220,7 @@ START_TEST(test_downloader_three_files_with_error) fail_if(handle == NULL); char *urls[] = {"http://www.google.com", NULL}; - lr_handle_setopt(handle, NULL, LRO_URLS, urls); + fail_if(!lr_handle_setopt(handle, NULL, LRO_URLS, urls)); lr_handle_prepare_internal_mirrorlist(handle, FALSE, &tmp_err); fail_if(tmp_err); @@ -230,12 +230,9 @@ START_TEST(test_downloader_three_files_with_error) tmpfn2 = lr_pathconcat(test_globals.tmpdir, "single_file_2_XXXXXX", NULL); tmpfn3 = lr_pathconcat(test_globals.tmpdir, "single_file_3_XXXXXX", NULL); - mktemp(tmpfn1); - mktemp(tmpfn2); - mktemp(tmpfn3); - fd1 = open(tmpfn1, O_RDWR|O_CREAT|O_TRUNC, 0666); - fd2 = open(tmpfn2, O_RDWR|O_CREAT|O_TRUNC, 0666); - fd3 = open(tmpfn3, O_RDWR|O_CREAT|O_TRUNC, 0666); + fd1 = mkstemp(tmpfn1); + fd2 = mkstemp(tmpfn2); + fd3 = mkstemp(tmpfn3); lr_free(tmpfn1); lr_free(tmpfn2); lr_free(tmpfn3); @@ -290,6 +287,9 @@ START_TEST(test_downloader_three_files_with_error) } g_slist_free_full(list, (GDestroyNotify) lr_downloadtarget_free); + close(fd1); + close(fd2); + close(fd3); } END_TEST @@ -331,7 +331,7 @@ START_TEST(test_downloader_checksum) fail_if(handle == NULL); char *urls[] = {"file:///", NULL}; - lr_handle_setopt(handle, NULL, LRO_URLS, urls); + fail_if(!lr_handle_setopt(handle, NULL, LRO_URLS, urls)); lr_handle_prepare_internal_mirrorlist(handle, FALSE, &tmp_err); fail_if(tmp_err); @@ -340,8 +340,7 @@ START_TEST(test_downloader_checksum) tmpfn1 = lr_pathconcat(test_globals.tmpdir, "single_file_XXXXXX", NULL); - mktemp(tmpfn1); - fd1 = open(tmpfn1, O_RDWR|O_CREAT|O_TRUNC, 0666); + fd1 = mkstemp(tmpfn1); lr_free(tmpfn1); fail_if(fd1 < 0); @@ -382,6 +381,7 @@ START_TEST(test_downloader_checksum) } g_slist_free_full(list, (GDestroyNotify) lr_downloadtarget_free); + close(fd1); } } END_TEST diff --git a/tests/test_repoconf.c b/tests/test_repoconf.c index 8a56b666..5c85047e 100644 --- a/tests/test_repoconf.c +++ b/tests/test_repoconf.c @@ -33,22 +33,6 @@ repoconf_assert_true(LrYumRepoConf *repoconf, #define conf_assert_true(option) \ repoconf_assert_true(conf, (option)) -static void -repoconf_assert_false(LrYumRepoConf *repoconf, - LrYumRepoConfOption option) -{ - ck_assert(1); - long val = 1L; - _cleanup_error_free_ GError *tmp_err = NULL; - gboolean ret = lr_yum_repoconf_getinfo(repoconf, &tmp_err, option, &val); - ck_assert(!tmp_err); - ck_assert(ret); - ck_assert_msg(!val, "Not a 0 (False) value (Option %d)", option); -} - -#define conf_assert_false(option) \ - repoconf_assert_false(conf, (option)) - static void repoconf_assert_na(LrYumRepoConf *repoconf, LrYumRepoConfOption option) @@ -509,7 +493,7 @@ END_TEST START_TEST(test_write_repoconf) { - _cleanup_fd_close_ int rc = -1; + _cleanup_fd_close_ int fd = -1; gboolean ret; LrYumRepoConfs *confs; LrYumRepoConf * conf; @@ -519,8 +503,8 @@ START_TEST(test_write_repoconf) _cleanup_error_free_ GError *tmp_err = NULL; // Create a temporary file - rc = mkstemp(tmpfn); - fail_if(rc == -1); + fd = mkstemp(tmpfn); + fail_if(fd == -1); // Create reconfs with one repoconf with one id (one section) confs = lr_yum_repoconfs_init();