From fd9d39cf19084dd4244943a3b27ee0a33805f76d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= <amatej@redhat.com>
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?= <amatej@redhat.com>
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();