From c1387013d2be0ac417368ff173578a6ff0690260 Mon Sep 17 00:00:00 2001
From: Florian Fischer <florian.fischer@muhq.space>
Date: Tue, 14 Sep 2021 16:11:56 +0200
Subject: [PATCH] [lib/LinuxVersion] multiple fixes
* remove fancy but broken static LinuxVersion object.
This means each usage of a EMPER_LINUX_* macro results in a syscall.
But I think this is worth it when the result is a sound program.
Users of those macros should store the result of the comparison anyway.
* fix that we skipped parsing the last part of a version string
* add a test for LinuxVersion
* IO_MUST_INVALIDATE_BROKEN_CHAIN can still be const
---
emper/Emper.cpp | 2 +-
emper/Emper.hpp | 2 +-
emper/lib/LinuxVersion.cpp | 22 ++++++----
emper/lib/LinuxVersion.hpp | 22 +++++-----
tests/lib/LinuxVersionTest.cpp | 75 ++++++++++++++++++++++++++++++++++
tests/lib/meson.build | 8 ++++
6 files changed, 109 insertions(+), 22 deletions(-)
create mode 100644 tests/lib/LinuxVersionTest.cpp
diff --git a/emper/Emper.cpp b/emper/Emper.cpp
index c9f6be84..72a6d51c 100644
--- a/emper/Emper.cpp
+++ b/emper/Emper.cpp
@@ -45,7 +45,7 @@ void async(const Fiber::fiber_fun0_t& function, workeraffinity_t* affinity) {
namespace emper {
-bool IO_MUST_INVALIDATE_BROKEN_CHAIN = EMPER_LINUX_LT("5.15.0");
+const bool IO_MUST_INVALIDATE_BROKEN_CHAIN = EMPER_LINUX_LT("5.15.0");
auto getFullVersion() -> std::string { return EMPER_FULL_VERSION; }
diff --git a/emper/Emper.hpp b/emper/Emper.hpp
index 78736f3c..13cb2c00 100644
--- a/emper/Emper.hpp
+++ b/emper/Emper.hpp
@@ -138,7 +138,7 @@ static const bool IO_URING_SQPOLL =
// Emper.cpp object.
// This also has the advantage that the probability we crash because printing
// warnings during the comparison use not yet initialized components is reduced.
-extern bool IO_MUST_INVALIDATE_BROKEN_CHAIN;
+extern const bool IO_MUST_INVALIDATE_BROKEN_CHAIN;
static const bool IO_URING_SHARED_WQ =
#ifdef EMPER_IO_URING_SHARED_WQ
diff --git a/emper/lib/LinuxVersion.cpp b/emper/lib/LinuxVersion.cpp
index f37e5f36..f268d050 100644
--- a/emper/lib/LinuxVersion.cpp
+++ b/emper/lib/LinuxVersion.cpp
@@ -33,19 +33,22 @@ static auto checked_strtol(const std::string& s) -> long {
namespace emper::lib {
-LinuxVersion LinuxVersion::linuxVersion;
-
// Returns 1 if s is smaller, -1 if this is smaller, 0 if equal
auto LinuxVersion::compare(const std::string& s) -> int {
+ size_t dot_pos, dot_pos2;
size_t last_dot_pos = 0, last_dot_pos2 = 0;
+ bool was_last = false;
for (;;) {
- size_t dot_pos = this->version.find('.', last_dot_pos);
- // We run out of parts to compare the versions must be equal
- if (dot_pos == std::string::npos) return 0;
-
- size_t dot_pos2 = s.find('.', last_dot_pos2);
- assert(dot_pos2 != std::string::npos);
+ dot_pos = this->version.find('.', last_dot_pos);
+ if (dot_pos == std::string::npos) {
+ was_last = true;
+ dot_pos = this->version.size();
+ dot_pos2 = s.size();
+ } else {
+ dot_pos2 = s.find('.', last_dot_pos2);
+ assert(dot_pos2 != std::string::npos);
+ }
long n1 = checked_strtol(this->version.substr(last_dot_pos, dot_pos - last_dot_pos));
long n2 = checked_strtol(s.substr(last_dot_pos2, dot_pos2 - last_dot_pos2));
@@ -54,6 +57,9 @@ auto LinuxVersion::compare(const std::string& s) -> int {
if (n1 < n2) return -1;
+ // We ran out of parts to compare the versions must be equal
+ if (was_last) return 0;
+
last_dot_pos = dot_pos + 1;
last_dot_pos2 = dot_pos2 + 1;
}
diff --git a/emper/lib/LinuxVersion.hpp b/emper/lib/LinuxVersion.hpp
index 6256cc40..ae437839 100644
--- a/emper/lib/LinuxVersion.hpp
+++ b/emper/lib/LinuxVersion.hpp
@@ -6,15 +6,18 @@
#include <string>
+class TestLinuxVersion;
+
namespace emper::lib {
class LinuxVersion {
+ friend class ::TestLinuxVersion;
std::string version;
auto compare(const std::string& s) -> int;
- public:
- static LinuxVersion linuxVersion;
+ LinuxVersion(std::string version) : version(version) {}
+ public:
LinuxVersion() {
struct utsname buf;
uname(&buf);
@@ -33,13 +36,8 @@ class LinuxVersion {
};
} // namespace emper::lib
-#define EMPER_LINUX_EQ(version) \
- ([]() -> bool { return empere::lib::LinuxVersion::linuxVersion == version; }())
-#define EMPER_LINUX_GT(version) \
- ([]() -> bool { return emper::lib::LinuxVersion::linuxVersion > version; }())
-#define EMPER_LINUX_GE(version) \
- ([]() -> bool { return emper::lib::LinuxVersion::linuxVersion >= version; }())
-#define EMPER_LINUX_LT(version) \
- ([]() -> bool { return emper::lib::LinuxVersion::linuxVersion < version; }())
-#define EMPER_LINUX_LE(version) \
- ([]() -> bool { return emper::lib::LinuxVersion::linuxVersion <= version; }())
+#define EMPER_LINUX_EQ(version) (emper::lib::LinuxVersion() == version)
+#define EMPER_LINUX_GT(version) (emper::lib::LinuxVersion() > version))
+#define EMPER_LINUX_GE(version) (emper::lib::LinuxVersion() >= version)
+#define EMPER_LINUX_LT(version) (emper::lib::LinuxVersion() < version)
+#define EMPER_LINUX_LE(version) (emper::lib::LinuxVersion() <= version)
diff --git a/tests/lib/LinuxVersionTest.cpp b/tests/lib/LinuxVersionTest.cpp
new file mode 100644
index 00000000..b14ec049
--- /dev/null
+++ b/tests/lib/LinuxVersionTest.cpp
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: LGPL-3.0-or-later
+// Copyright © 2021 Florian Fischer
+#include <gtest/gtest.h>
+
+#include <string>
+#include <utility>
+
+#include "lib/LinuxVersion.hpp"
+
+using emper::lib::LinuxVersion;
+
+class TestLinuxVersion {
+ public:
+ static auto eq(std::string v1, const std::string& v2) -> bool {
+ return LinuxVersion(std::move(v1)) == v2;
+ }
+ static auto neq(std::string v1, const std::string& v2) -> bool {
+ return LinuxVersion(std::move(v1)) != v2;
+ }
+ static auto lt(std::string v1, const std::string& v2) -> bool {
+ return LinuxVersion(std::move(v1)) < v2;
+ }
+ static auto le(std::string v1, const std::string& v2) -> bool {
+ return LinuxVersion(std::move(v1)) <= v2;
+ }
+ static auto gt(std::string v1, const std::string& v2) -> bool {
+ return LinuxVersion(std::move(v1)) > v2;
+ }
+ static auto ge(std::string v1, const std::string& v2) -> bool {
+ return LinuxVersion(std::move(v1)) >= v2;
+ }
+};
+
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
+TEST(LinuxVersion, eq) {
+ ASSERT_TRUE(TestLinuxVersion::eq("5.13.0", "5.13.0"));
+ ASSERT_TRUE(TestLinuxVersion::eq("5.13-foo.0", "5.13.0"));
+}
+
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
+TEST(LinuxVersion, neq) {
+ ASSERT_TRUE(TestLinuxVersion::neq("4.1.0", "5.13.0"));
+ ASSERT_TRUE(TestLinuxVersion::neq("4.1", "5.13.0"));
+ ASSERT_TRUE(TestLinuxVersion::neq("5.14.0", "5.13.0"));
+ ASSERT_TRUE(TestLinuxVersion::neq("5.13.1", "5.13.2"));
+ ASSERT_TRUE(TestLinuxVersion::neq("5.13-foo.1", "5.13.2"));
+}
+
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
+TEST(LinuxVersion, lt) {
+ ASSERT_TRUE(TestLinuxVersion::lt("4.13", "5.14.0"));
+ ASSERT_TRUE(TestLinuxVersion::lt("5.13.0", "5.14.0"));
+ ASSERT_TRUE(TestLinuxVersion::lt("5.13-foo.1", "5.13.2"));
+ ASSERT_TRUE(TestLinuxVersion::lt("5.13.1-foo", "5.13.2"));
+}
+
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
+TEST(LinuxVersion, le) {
+ ASSERT_TRUE(TestLinuxVersion::le("4.13.0", "5.14.0"));
+ ASSERT_TRUE(TestLinuxVersion::le("4.14.0", "5.14.0"));
+}
+
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
+TEST(LinuxVersion, gt) {
+ ASSERT_TRUE(TestLinuxVersion::gt("5.14.0", "4.13"));
+ ASSERT_TRUE(TestLinuxVersion::gt("5.14.0", "5.13.0"));
+ ASSERT_TRUE(TestLinuxVersion::gt("5.13.2", "5.13-foo.1"));
+ ASSERT_TRUE(TestLinuxVersion::gt("5.13.2", "5.13.1-foo"));
+}
+
+// NOLINTNEXTLINE(modernize-use-trailing-return-type)
+TEST(LinuxVersion, ge) {
+ ASSERT_TRUE(TestLinuxVersion::ge("5.14.0", "4.13"));
+ ASSERT_TRUE(TestLinuxVersion::ge("5.14.0", "5.14.0"));
+}
diff --git a/tests/lib/meson.build b/tests/lib/meson.build
index 76e0ecd7..1bba5e40 100644
--- a/tests/lib/meson.build
+++ b/tests/lib/meson.build
@@ -13,4 +13,12 @@ tests += [
'gtest': true,
'is_parallel': true,
},
+
+ {
+ 'source': files('LinuxVersionTest.cpp'),
+ 'name': 'LinuxVersionTest',
+ 'description': 'Tests LinuxVerison',
+ 'gtest': true,
+ 'is_parallel': true,
+ },
]
--
GitLab