Fix static variable initialization order issues

Static variables from different compilation units are initialized in
undefined order. This might cause problems if one variable depends on
another. This commit removes such problematic dependencies.
Additionally, in some places unnecessary static std::string variables are
removed to avoid potential problems in the future.

Change-Id: I32050f8774571e0d1cfc5a588f4dbe470a9ef1c9
Signed-off-by: Radoslaw Bartosiak <r.bartosiak@samsung.com>

diff --git a/src/common/types/PolicyBucket.cpp b/src/common/types/PolicyBucket.cpp
index a25c274..a20044d 100644 (file)
--- a/src/common/types/PolicyBucket.cpp
+++ b/src/common/types/PolicyBucket.cpp
@@ -20,6 +20,7 @@
  * @brief       Implementation of Cynara::PolicyBucket methods
  */
 
+#include <cstring>
 #include <sstream>
 
 #include <exceptions/InvalidBucketIdException.h>
@@ -30,7 +31,7 @@
 
 namespace Cynara {
 
-const std::string PolicyBucket::m_idSeparators = "-_";
+const char PolicyBucket::m_idSeparators[] = "-_";
 
 PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyResult &defaultPolicy)
     : m_defaultPolicy(defaultPolicy), m_id(id) {
@@ -129,7 +130,7 @@ void PolicyBucket::idValidator(const PolicyBucketId &id) {
 }
 
 bool PolicyBucket::isIdSeparator(char c) {
-    return m_idSeparators.end() != find(m_idSeparators.begin(), m_idSeparators.end(), c);
+    return strchr(m_idSeparators, c) != nullptr;
 }
 
 }  // namespace Cynara

diff --git a/src/common/types/PolicyBucket.h b/src/common/types/PolicyBucket.h
index 62cdf5f..029d3dd 100644 (file)
--- a/src/common/types/PolicyBucket.h
+++ b/src/common/types/PolicyBucket.h
@@ -110,7 +110,7 @@ private:
     PolicyMap m_policyCollection;
     PolicyResult m_defaultPolicy;
     PolicyBucketId m_id;
-    static const std::string m_idSeparators;
+    static const char m_idSeparators[];
 };
 
 } /* namespace Cynara */

diff --git a/src/helpers/creds-dbus/creds-dbus-inner.cpp b/src/helpers/creds-dbus/creds-dbus-inner.cpp
index fc48217..5ea657e 100644 (file)
--- a/src/helpers/creds-dbus/creds-dbus-inner.cpp
+++ b/src/helpers/creds-dbus/creds-dbus-inner.cpp
@@ -42,8 +42,8 @@ public:
         if (connection == nullptr)
             throw CYNARA_API_INVALID_PARAM;
 
-        m_message = dbus_message_new_method_call(m_dbusName.c_str(), m_dbusObject.c_str(),
-                                                 m_dbusInterface.c_str(), method.c_str());
+        m_message = dbus_message_new_method_call(m_dbusName, m_dbusObject,
+                                                 m_dbusInterface, method.c_str());
 
         if (m_message == nullptr)
             throw CYNARA_API_OUT_OF_MEMORY;
@@ -104,14 +104,14 @@ private:
     DBusMessage *m_message = nullptr;
     DBusMessageIter *m_argsIter = nullptr;
 
-    static const std::string m_dbusName;
-    static const std::string m_dbusObject;
-    static const std::string m_dbusInterface;
+    static const char m_dbusName[];
+    static const char m_dbusObject[];
+    static const char m_dbusInterface[];
 };
 
-const std::string DBusMethod::m_dbusName = "org.freedesktop.DBus";
-const std::string DBusMethod::m_dbusObject = "/org/freedesktop/DBus";
-const std::string DBusMethod::m_dbusInterface = "org.freedesktop.DBus";
+const char DBusMethod::m_dbusName[] = "org.freedesktop.DBus";
+const char DBusMethod::m_dbusObject[] = "/org/freedesktop/DBus";
+const char DBusMethod::m_dbusInterface[] = "org.freedesktop.DBus";
 
 
 int getIdFromConnection(DBusConnection *connection, const char *uniqueName,

diff --git a/src/storage/ChecksumValidator.cpp b/src/storage/ChecksumValidator.cpp
index 1354ad1..328f98c 100644 (file)
--- a/src/storage/ChecksumValidator.cpp
+++ b/src/storage/ChecksumValidator.cpp
@@ -38,10 +38,6 @@
 
 namespace Cynara {
 
-const std::string ChecksumValidator::m_checksumFilename(PathConfig::StoragePath::checksumFilename);
-const std::string ChecksumValidator::m_backupFilenameSuffix(
-        PathConfig::StoragePath::backupFilenameSuffix);
-
 void ChecksumValidator::load(std::istream &stream) {
     m_sums.clear();
 
@@ -94,10 +90,11 @@ void ChecksumValidator::compare(std::istream &stream, const std::string &pathnam
     std::stringstream copyStream;
 
     if (isBackupValid) {
-        auto backupSuffixPos = filename.rfind(m_backupFilenameSuffix);
+        auto backupSuffixPos = filename.rfind(PathConfig::StoragePath::backupFilenameSuffix);
+        size_t suffixSize = PathConfig::StoragePath::backupFilenameSuffix.size();
 
         if ((std::string::npos != backupSuffixPos) &&
-            (filename.size() == (backupSuffixPos + m_backupFilenameSuffix.size()))) {
+            (filename.size() == (backupSuffixPos + suffixSize))) {
             filename.erase(backupSuffixPos);
         }
     }
@@ -135,8 +132,9 @@ const std::string ChecksumValidator::parseChecksum(const std::string &line,
 }
 
 bool ChecksumValidator::isChecksumIndex(const std::string &filename) const {
-    auto checksum = m_dbPath + m_checksumFilename;
-    return (filename == checksum || filename == checksum + m_backupFilenameSuffix);
+    auto checksum = m_dbPath + PathConfig::StoragePath::checksumFilename;
+    return (filename == checksum ||
+            filename == checksum + PathConfig::StoragePath::backupFilenameSuffix);
 }
 
 } // namespace Cynara

diff --git a/src/storage/ChecksumValidator.h b/src/storage/ChecksumValidator.h
index bfd577b..affaa64 100644 (file)
--- a/src/storage/ChecksumValidator.h
+++ b/src/storage/ChecksumValidator.h
@@ -56,8 +56,6 @@ protected:
 
     Checksums m_sums;
     const std::string m_dbPath;
-    static const std::string m_checksumFilename;
-    static const std::string m_backupFilenameSuffix;
 };
 
 } // namespace Cynara

diff --git a/src/storage/InMemoryStorageBackend.cpp b/src/storage/InMemoryStorageBackend.cpp
index 15f6a8a..a6f43ee 100644 (file)
--- a/src/storage/InMemoryStorageBackend.cpp
+++ b/src/storage/InMemoryStorageBackend.cpp
@@ -53,13 +53,6 @@
 
 namespace Cynara {
 
-const std::string InMemoryStorageBackend::m_chsFilename(PathConfig::StoragePath::checksumFilename);
-const std::string InMemoryStorageBackend::m_indexFilename(PathConfig::StoragePath::indexFilename);
-const std::string InMemoryStorageBackend::m_backupFilenameSuffix(
-        PathConfig::StoragePath::backupFilenameSuffix);
-const std::string InMemoryStorageBackend::m_bucketFilenamePrefix(
-        PathConfig::StoragePath::bucketFilenamePrefix);
-
 InMemoryStorageBackend::InMemoryStorageBackend(const std::string &path) : m_dbPath(path),
     m_checksum(path), m_integrity(path) {
 }
@@ -67,13 +60,13 @@ InMemoryStorageBackend::InMemoryStorageBackend(const std::string &path) : m_dbPa
 void InMemoryStorageBackend::load(void) {
     bool isBackupValid = m_integrity.backupGuardExists();
     std::string bucketSuffix = "";
-    std::string indexFilename = m_dbPath + m_indexFilename;
-    std::string chsFilename = m_dbPath + m_chsFilename;
+    std::string indexFilename = m_dbPath + PathConfig::StoragePath::indexFilename;
+    std::string chsFilename = m_dbPath + PathConfig::StoragePath::checksumFilename;
 
     if (isBackupValid) {
-        bucketSuffix += m_backupFilenameSuffix;
-        indexFilename += m_backupFilenameSuffix;
-        chsFilename += m_backupFilenameSuffix;
+        bucketSuffix += PathConfig::StoragePath::backupFilenameSuffix;
+        indexFilename += PathConfig::StoragePath::backupFilenameSuffix;
+        chsFilename += PathConfig::StoragePath::backupFilenameSuffix;
     }
 
     try {
@@ -106,9 +99,10 @@ void InMemoryStorageBackend::load(void) {
 }
 
 void InMemoryStorageBackend::save(void) {
-    std::string checksumFilename = m_dbPath + m_chsFilename;
+    std::string checksumFilename = m_dbPath + PathConfig::StoragePath::checksumFilename;
     auto chsStream = std::make_shared<std::ofstream>();
-    openDumpFileStream<std::ofstream>(*chsStream, checksumFilename + m_backupFilenameSuffix);
+    openDumpFileStream<std::ofstream>(*chsStream,
+            checksumFilename + PathConfig::StoragePath::backupFilenameSuffix);
 
     dumpDatabase(chsStream);
 
@@ -230,9 +224,11 @@ void InMemoryStorageBackend::erasePolicies(const PolicyBucketId &bucketId, bool
 }
 
 void InMemoryStorageBackend::dumpDatabase(const std::shared_ptr<std::ofstream> &chsStream) {
-    auto indexStream = std::make_shared<ChecksumStream>(m_indexFilename, chsStream);
-    std::string indexFilename = m_dbPath + m_indexFilename;
-    openDumpFileStream<ChecksumStream>(*indexStream, indexFilename + m_backupFilenameSuffix);
+    auto indexStream = std::make_shared<ChecksumStream>(PathConfig::StoragePath::indexFilename,
+            chsStream);
+    std::string indexFilename = m_dbPath + PathConfig::StoragePath::indexFilename;
+    openDumpFileStream<ChecksumStream>(*indexStream,
+            indexFilename + PathConfig::StoragePath::backupFilenameSuffix);
 
     StorageSerializer<ChecksumStream> storageSerializer(indexStream);
     storageSerializer.dump(buckets(), std::bind(&InMemoryStorageBackend::bucketDumpStreamOpener,
@@ -254,7 +250,8 @@ void InMemoryStorageBackend::openFileStream(std::ifstream &stream, const std::st
 
 std::shared_ptr<BucketDeserializer> InMemoryStorageBackend::bucketStreamOpener(
         const PolicyBucketId &bucketId, const std::string &filenameSuffix, bool isBackupValid) {
-    std::string bucketFilename = m_dbPath + m_bucketFilenamePrefix + bucketId + filenameSuffix;
+    std::string bucketFilename = m_dbPath + PathConfig::StoragePath::bucketFilenamePrefix +
+            bucketId + filenameSuffix;
     auto bucketStream = std::make_shared<std::ifstream>();
     try {
         openFileStream(*bucketStream, bucketFilename, isBackupValid);
@@ -268,10 +265,10 @@ std::shared_ptr<BucketDeserializer> InMemoryStorageBackend::bucketStreamOpener(
 
 std::shared_ptr<StorageSerializer<ChecksumStream> > InMemoryStorageBackend::bucketDumpStreamOpener(
         const PolicyBucketId &bucketId, const std::shared_ptr<std::ofstream> &chsStream) {
-    std::string bucketFilename = m_dbPath + m_bucketFilenamePrefix +
-                                 bucketId + m_backupFilenameSuffix;
-    auto bucketStream = std::make_shared<ChecksumStream>(m_bucketFilenamePrefix + bucketId,
-                                                         chsStream);
+    std::string bucketFilename = m_dbPath + PathConfig::StoragePath::bucketFilenamePrefix +
+                                 bucketId + PathConfig::StoragePath::backupFilenameSuffix;
+    auto bucketStream = std::make_shared<ChecksumStream>(
+            PathConfig::StoragePath::bucketFilenamePrefix + bucketId, chsStream);
 
     openDumpFileStream<ChecksumStream>(*bucketStream, bucketFilename);
     return std::make_shared<StorageSerializer<ChecksumStream> >(bucketStream);

diff --git a/src/storage/InMemoryStorageBackend.h b/src/storage/InMemoryStorageBackend.h
index 80a505c..99a7f02 100644 (file)
--- a/src/storage/InMemoryStorageBackend.h
+++ b/src/storage/InMemoryStorageBackend.h
@@ -86,10 +86,6 @@ private:
     Buckets m_buckets;
     ChecksumValidator m_checksum;
     Integrity m_integrity;
-    static const std::string m_chsFilename;
-    static const std::string m_indexFilename;
-    static const std::string m_backupFilenameSuffix;
-    static const std::string m_bucketFilenamePrefix;
 
 protected:
     virtual Buckets &buckets(void) {

diff --git a/src/storage/Integrity.cpp b/src/storage/Integrity.cpp
index 1735b9e..f855915 100644 (file)
--- a/src/storage/Integrity.cpp
+++ b/src/storage/Integrity.cpp
@@ -41,14 +41,9 @@ namespace Cynara {
 
 namespace StorageConfig = PathConfig::StoragePath;
 
-const std::string Integrity::m_guardFilename(StorageConfig::guardFilename);
-const std::string Integrity::m_indexFilename(StorageConfig::indexFilename);
-const std::string Integrity::m_backupFilenameSuffix(StorageConfig::backupFilenameSuffix);
-const std::string Integrity::m_bucketFilenamePrefix(StorageConfig::bucketFilenamePrefix);
-
 bool Integrity::backupGuardExists(void) const {
     struct stat buffer;
-    std::string guardFilename = m_dbPath + m_guardFilename;
+    std::string guardFilename = m_dbPath + StorageConfig::guardFilename;
 
     int ret = stat(guardFilename.c_str(), &buffer);
 
@@ -65,7 +60,7 @@ bool Integrity::backupGuardExists(void) const {
 }
 
 void Integrity::createBackupGuard(void) const {
-    syncElement(m_dbPath + m_guardFilename, O_CREAT | O_EXCL | O_WRONLY | O_TRUNC);
+    syncElement(m_dbPath + StorageConfig::guardFilename, O_CREAT | O_EXCL | O_WRONLY | O_TRUNC);
     syncDirectory(m_dbPath);
 }
 
@@ -73,17 +68,18 @@ void Integrity::syncDatabase(const Buckets &buckets, bool syncBackup) {
     std::string suffix = "";
 
     if (syncBackup) {
-        suffix += m_backupFilenameSuffix;
+        suffix += StorageConfig::backupFilenameSuffix;
     }
 
     for (const auto &bucketIter : buckets) {
         const auto &bucketId = bucketIter.first;
-        const auto &bucketFilename = m_dbPath + m_bucketFilenamePrefix + bucketId + suffix;
+        const auto &bucketFilename = m_dbPath + StorageConfig::bucketFilenamePrefix +
+                bucketId + suffix;
 
         syncElement(bucketFilename);
     }
 
-    syncElement(m_dbPath + m_indexFilename + suffix);
+    syncElement(m_dbPath + StorageConfig::indexFilename + suffix);
     syncElement(m_dbPath + PathConfig::StoragePath::checksumFilename + suffix);
     syncDirectory(m_dbPath);
 }
@@ -92,7 +88,7 @@ void Integrity::revalidatePrimaryDatabase(const Buckets &buckets) {
     createPrimaryHardLinks(buckets);
     syncDatabase(buckets, false);
 
-    deleteHardLink(m_dbPath + m_guardFilename);
+    deleteHardLink(m_dbPath + StorageConfig::guardFilename);
     syncDirectory(m_dbPath);
 
     deleteBackupHardLinks(buckets);
@@ -127,7 +123,7 @@ void Integrity::deleteNonIndexedFiles(BucketPresenceTester tester) {
 
         std::string bucketId;
         auto nameLength = filename.length();
-        auto prefixLength = m_bucketFilenamePrefix.length();
+        auto prefixLength = StorageConfig::bucketFilenamePrefix.length();
 
         //remove if it is impossible that it is a bucket file
         if (nameLength < prefixLength) {
@@ -137,7 +133,7 @@ void Integrity::deleteNonIndexedFiles(BucketPresenceTester tester) {
 
         //remove if there is no bucket filename prefix
         //0 is returned from string::compare() if strings are equal
-        if (0 != filename.compare(0, prefixLength, m_bucketFilenamePrefix)) {
+        if (0 != filename.compare(0, prefixLength, StorageConfig::bucketFilenamePrefix)) {
             deleteHardLink(m_dbPath + filename);
             continue;
         }
@@ -199,32 +195,33 @@ void Integrity::syncDirectory(const std::string &dirname, mode_t mode) {
 void Integrity::createPrimaryHardLinks(const Buckets &buckets) {
     for (const auto &bucketIter : buckets) {
         const auto &bucketId = bucketIter.first;
-        const auto &bucketFilename = m_dbPath + m_bucketFilenamePrefix + bucketId;
+        const auto &bucketFilename = m_dbPath + StorageConfig::bucketFilenamePrefix + bucketId;
 
         deleteHardLink(bucketFilename);
-        createHardLink(bucketFilename + m_backupFilenameSuffix, bucketFilename);
+        createHardLink(bucketFilename + StorageConfig::backupFilenameSuffix, bucketFilename);
     }
 
-    const auto &indexFilename = m_dbPath + m_indexFilename;
+    const auto &indexFilename = m_dbPath + StorageConfig::indexFilename;
     const auto &checksumFilename = m_dbPath + PathConfig::StoragePath::checksumFilename;
 
     deleteHardLink(indexFilename);
-    createHardLink(indexFilename + m_backupFilenameSuffix, indexFilename);
+    createHardLink(indexFilename + StorageConfig::backupFilenameSuffix, indexFilename);
     deleteHardLink(checksumFilename);
-    createHardLink(checksumFilename + m_backupFilenameSuffix, checksumFilename);
+    createHardLink(checksumFilename + StorageConfig::backupFilenameSuffix, checksumFilename);
 }
 
 void Integrity::deleteBackupHardLinks(const Buckets &buckets) {
     for (const auto &bucketIter : buckets) {
         const auto &bucketId = bucketIter.first;
-        const auto &bucketFilename = m_dbPath + m_bucketFilenamePrefix +
-                                     bucketId + m_backupFilenameSuffix;
+        const auto &bucketFilename = m_dbPath + StorageConfig::bucketFilenamePrefix +
+                                     bucketId + StorageConfig::backupFilenameSuffix;
 
         deleteHardLink(bucketFilename);
     }
 
-    deleteHardLink(m_dbPath + m_indexFilename + m_backupFilenameSuffix);
-    deleteHardLink(m_dbPath + PathConfig::StoragePath::checksumFilename + m_backupFilenameSuffix);
+    deleteHardLink(m_dbPath + StorageConfig::indexFilename + StorageConfig::backupFilenameSuffix);
+    deleteHardLink(m_dbPath + StorageConfig::checksumFilename +
+            StorageConfig::backupFilenameSuffix);
 }
 
 void Integrity::createHardLink(const std::string &oldName, const std::string &newName) {

diff --git a/src/storage/Integrity.h b/src/storage/Integrity.h
index 569ca85..3c3d50a 100644 (file)
--- a/src/storage/Integrity.h
+++ b/src/storage/Integrity.h
@@ -60,10 +60,6 @@ protected:
 
 private:
     const std::string m_dbPath;
-    static const std::string m_indexFilename;
-    static const std::string m_backupFilenameSuffix;
-    static const std::string m_bucketFilenamePrefix;
-    static const std::string m_guardFilename;
 };
 
 } // namespace Cynara

diff --git a/test/chsgen/checksumgenerator.cpp b/test/chsgen/checksumgenerator.cpp
index 5fcd03a..9ecfc24 100644 (file)
--- a/test/chsgen/checksumgenerator.cpp
+++ b/test/chsgen/checksumgenerator.cpp
@@ -33,7 +33,7 @@
 namespace {
 
 const std::string execName("./cynara-db-chsgen");
-const std::string backupFilenameSuffix(Cynara::PathConfig::StoragePath::backupFilenameSuffix);
+const std::string &backupFilenameSuffix = Cynara::PathConfig::StoragePath::backupFilenameSuffix;
 const char fieldSeparator(Cynara::PathConfig::StoragePath::fieldSeparator);
 
 } // namespace

diff --git a/test/storage/checksum/checksumvalidator.cpp b/test/storage/checksum/checksumvalidator.cpp
index 19918f3..7145f37 100644 (file)
--- a/test/storage/checksum/checksumvalidator.cpp
+++ b/test/storage/checksum/checksumvalidator.cpp
@@ -41,8 +41,7 @@ using namespace Cynara;
 const std::string ChecksumValidatorFixture::m_dbPath("/fake/path/");
 const std::string ChecksumValidatorFixture::m_filename("fakeFilename");
 const std::string ChecksumValidatorFixture::m_checksum("$1$$fakeChecksum");
-const std::string ChecksumValidatorFixture::m_backupFilenameSuffix(
-        PathConfig::StoragePath::backupFilenameSuffix);
+
 const char ChecksumValidatorFixture::m_fieldSeparator(PathConfig::StoragePath::fieldSeparator);
 const char ChecksumValidatorFixture::m_recordSeparator(PathConfig::StoragePath::recordSeparator);
 const size_t ChecksumValidatorFixture::m_firstLine(1);
@@ -120,7 +119,8 @@ TEST_F(ChecksumValidatorFixture, compareBasicAndBackup) {
         SCOPED_TRACE(filename);
 
         ASSERT_NO_THROW(validator.compare(fakeFile, filename, false));
-        ASSERT_NO_THROW(validator.compare(fakeFile, filename + m_backupFilenameSuffix, true));
+        ASSERT_NO_THROW(validator.compare(fakeFile, filename +
+                PathConfig::StoragePath::backupFilenameSuffix, true));
 
         ASSERT_EQ(contents, fakeFile.str());
     }

diff --git a/test/storage/checksum/checksumvalidatorfixture.h b/test/storage/checksum/checksumvalidatorfixture.h
index 6fb28f6..b50cb2b 100644 (file)
--- a/test/storage/checksum/checksumvalidatorfixture.h
+++ b/test/storage/checksum/checksumvalidatorfixture.h
@@ -45,7 +45,6 @@ protected:
     static const std::string m_dbPath;
     static const std::string m_filename;
     static const std::string m_checksum;
-    static const std::string m_backupFilenameSuffix;
     static const char m_fieldSeparator;
     static const char m_recordSeparator;
     static const size_t m_firstLine;