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();