From 916d418824586fd4f11d57356410492f779547b4 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Fri, 1 Oct 2021 15:28:01 -0500 Subject: [PATCH] fix: some Coverity regressions reported on 2021-09-16 (#1870) * fix: coverity regressions reported on 2021-10-01 leaks introduced by 3fd5c81a * fix: resource leak in test utility filesAreIdentical() * fix: use-after-free warning in test code for tr_urlParse() * fix: false warning for unterminated c string * fix: false unchecked return value in test code cid 1491881 * fix: unterminated c string in test code cid 1491890 * refactor: slightly better assertions in file-test --- tests/libtransmission/blocklist-test.cc | 3 +-- tests/libtransmission/copy-test.cc | 10 +++++++--- tests/libtransmission/file-test.cc | 8 ++++---- tests/libtransmission/subprocess-test.cc | 10 ++++++++++ tests/libtransmission/utils-test.cc | 12 +++++++++--- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/tests/libtransmission/blocklist-test.cc b/tests/libtransmission/blocklist-test.cc index b9cb1c6fe..7b2c2707d 100644 --- a/tests/libtransmission/blocklist-test.cc +++ b/tests/libtransmission/blocklist-test.cc @@ -66,8 +66,7 @@ protected: bool addressIsBlocked(char const* address_str) { struct tr_address addr = {}; - tr_address_from_string(&addr, address_str); - return tr_sessionIsAddressBlocked(session_, &addr); + return !tr_address_from_string(&addr, address_str) || tr_sessionIsAddressBlocked(session_, &addr); } }; diff --git a/tests/libtransmission/copy-test.cc b/tests/libtransmission/copy-test.cc index ee0c36360..4cafb5158 100644 --- a/tests/libtransmission/copy-test.cc +++ b/tests/libtransmission/copy-test.cc @@ -75,6 +75,8 @@ private: bool filesAreIdentical(char const* fn1, char const* fn2) { + bool identical = true; + tr_sys_file_t fd1 = tr_sys_file_open(fn1, TR_SYS_FILE_READ | TR_SYS_FILE_SEQUENTIAL, 0, nullptr); tr_sys_file_t fd2 = tr_sys_file_open(fn2, TR_SYS_FILE_READ | TR_SYS_FILE_SEQUENTIAL, 0, nullptr); EXPECT_NE(fd1, TR_BAD_SYS_FILE); @@ -100,12 +102,14 @@ private: if (bytes_left1 != bytes_left2) { - return false; + identical = false; + break; } if (memcmp(readbuf1, readbuf2, buflen) != 0) { - return false; + identical = false; + break; } } @@ -114,7 +118,7 @@ private: tr_sys_file_close(fd1, nullptr); tr_sys_file_close(fd2, nullptr); - return true; + return identical; } }; diff --git a/tests/libtransmission/file-test.cc b/tests/libtransmission/file-test.cc index 67b05bcbf..8581887c9 100644 --- a/tests/libtransmission/file-test.cc +++ b/tests/libtransmission/file-test.cc @@ -999,11 +999,11 @@ TEST_F(FileTest, fileOpen) auto* path1 = tr_buildPath(test_dir.data(), "a", nullptr); EXPECT_FALSE(tr_sys_path_exists(path1, nullptr)); tr_error* err = nullptr; - EXPECT_TRUE(tr_sys_file_open(path1, TR_SYS_FILE_READ, 0600, &err) == TR_BAD_SYS_FILE); + EXPECT_EQ(TR_BAD_SYS_FILE, tr_sys_file_open(path1, TR_SYS_FILE_READ, 0600, &err)); EXPECT_NE(nullptr, err); EXPECT_FALSE(tr_sys_path_exists(path1, nullptr)); tr_error_clear(&err); - EXPECT_TRUE(tr_sys_file_open(path1, TR_SYS_FILE_WRITE, 0600, &err) == TR_BAD_SYS_FILE); + EXPECT_EQ(TR_BAD_SYS_FILE, tr_sys_file_open(path1, TR_SYS_FILE_WRITE, 0600, &err)); EXPECT_NE(nullptr, err); EXPECT_FALSE(tr_sys_path_exists(path1, nullptr)); tr_error_clear(&err); @@ -1012,11 +1012,11 @@ TEST_F(FileTest, fileOpen) tr_sys_dir_create(path1, 0, 0777, nullptr); #ifdef _WIN32 // this works on *NIX - EXPECT_TRUE(tr_sys_file_open(path1, TR_SYS_FILE_READ, 0600, &err) == TR_BAD_SYS_FILE); + EXPECT_EQ(TR_BAD_SYS_FILE, tr_sys_file_open(path1, TR_SYS_FILE_READ, 0600, &err)); EXPECT_NE(nullptr, err); tr_error_clear(&err); #endif - EXPECT_TRUE(tr_sys_file_open(path1, TR_SYS_FILE_WRITE, 0600, &err) == TR_BAD_SYS_FILE); + EXPECT_EQ(TR_BAD_SYS_FILE, tr_sys_file_open(path1, TR_SYS_FILE_WRITE, 0600, &err)); EXPECT_NE(nullptr, err); tr_error_clear(&err); diff --git a/tests/libtransmission/subprocess-test.cc b/tests/libtransmission/subprocess-test.cc index 769226468..a4fa16985 100644 --- a/tests/libtransmission/subprocess-test.cc +++ b/tests/libtransmission/subprocess-test.cc @@ -137,17 +137,21 @@ TEST_P(SubprocessTest, SpawnAsyncArgs) auto buffer = std::array{}; + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_arg1, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_arg2, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_arg3, buffer.data()); if (allow_batch_metachars) { + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_arg4, buffer.data()); } @@ -212,21 +216,27 @@ TEST_P(SubprocessTest, SpawnAsyncEnv) auto buffer = std::array{}; + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_env_value1, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_env_value2, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_env_value3, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_env_value4, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_EQ(test_env_value5, buffer.data()); + buffer[0] = '\0'; EXPECT_TRUE(tr_sys_file_read_line(fd, buffer.data(), buffer.size(), nullptr)); EXPECT_STREQ("", buffer.data()); diff --git a/tests/libtransmission/utils-test.cc b/tests/libtransmission/utils-test.cc index 69fbf7f4b..5c73ff258 100644 --- a/tests/libtransmission/utils-test.cc +++ b/tests/libtransmission/utils-test.cc @@ -225,9 +225,9 @@ TEST_F(UtilsTest, url) { auto const* url = "http://1"; int port; - char* scheme; - char* host; - char* path; + char* scheme = nullptr; + char* host = nullptr; + char* path = nullptr; EXPECT_TRUE(tr_urlParse(url, TR_BAD_SIZE, &scheme, &host, &port, &path)); EXPECT_STREQ("http", scheme); EXPECT_STREQ("1", host); @@ -238,6 +238,9 @@ TEST_F(UtilsTest, url) tr_free(host); url = "http://www.some-tracker.org/some/path"; + scheme = nullptr; + host = nullptr; + path = nullptr; EXPECT_TRUE(tr_urlParse(url, TR_BAD_SIZE, &scheme, &host, &port, &path)); EXPECT_STREQ("http", scheme); EXPECT_STREQ("www.some-tracker.org", host); @@ -248,6 +251,9 @@ TEST_F(UtilsTest, url) tr_free(host); url = "http://www.some-tracker.org:8080/some/path"; + scheme = nullptr; + host = nullptr; + path = nullptr; EXPECT_TRUE(tr_urlParse(url, TR_BAD_SIZE, &scheme, &host, &port, &path)); EXPECT_STREQ("http", scheme); EXPECT_STREQ("www.some-tracker.org", host);