refactoring: hide XmlParser

This disconnects XmlParser from __internal_init() and hides it
behind StorageBackendXML. This way XmlParser is local, non static and
non global anymore, a bit more tightly connected to StorageBackendXML,
but more loosely connected to other components.

Change-Id: I712129dfc9f8da357e7a7d66411aaea3590172e7

diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp
index 82757ec..d5dcd22 100644 (file)
--- a/src/internal/internal.cpp
+++ b/src/internal/internal.cpp
@@ -36,11 +36,10 @@ static const char* get_str(const char* const szstr) {
 
 int __internal_init(BusType bus_type, const char* const config_name)
 {
-       policy_checker().clearDb(bus_type);
-       auto err = static_parser().parsePolicyConfigFile(bus_type, get_str(config_name));
+       auto ok = policy_checker().initDb(bus_type, get_str(config_name));
        if (tslog::enabled())
                memory_dump(bus_type);
-       return err;
+       return ok ? 0 : -1;
 }
 
 pthread_mutex_t g_mutex = PTHREAD_MUTEX_INITIALIZER;

diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp
index f35bd11..ab7f78f 100644 (file)
--- a/src/internal/naive_policy_checker.cpp
+++ b/src/internal/naive_policy_checker.cpp
@@ -120,9 +120,9 @@ void NaivePolicyChecker::updateGroupDb(BusType bus_type, uid_t uid, gid_t gid)
        getPolicyDb(bus_type).initializeGroups(uid, gid);
 }
 
-void NaivePolicyChecker::clearDb(BusType bus_type)
+bool NaivePolicyChecker::initDb(BusType bus_type, const char *config_name)
 {
-       getPolicyDb(bus_type).clear();
+       return getPolicyDb(bus_type).init(config_name);
 }
 
 void NaivePolicyChecker::printContent(BusType bus_type)

diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp
index b9da290..e1a6e5a 100644 (file)
--- a/src/internal/naive_policy_checker.hpp
+++ b/src/internal/naive_policy_checker.hpp
@@ -88,7 +88,7 @@ namespace ldp_xml_parser
                /** Clears all db data, useful for reloading configuration
                 * during testing.
                 */
-               void clearDb(BusType bus_type);
+               bool initDb(BusType bus_type, const char *filename);
 
                void updateGroupDb(BusType bus_type, uid_t uid, gid_t gid);
 

diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp
index cf53c10..08e95bc 100644 (file)
--- a/src/internal/naive_policy_db.cpp
+++ b/src/internal/naive_policy_db.cpp
@@ -57,14 +57,6 @@ void NaivePolicyDb::updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid,
 }
 
 /********************* NaivePolicyDb **********************/
-template <typename T>
-void NaivePolicyDb::addItem(const PolicyType policy_type,
-                                                       const PolicyTypeValue policy_type_value,
-                                                       T &item) {
-       tslog::log("Add item: ", item, ", decision: ", item.getDecision(), "\n");
-       backend.addItem(policy_type, policy_type_value, item);
-}
-
 void NaivePolicyDb::printContent() const
 {
        backend.printContent();
@@ -136,8 +128,8 @@ VGid &NaivePolicyDb::getMapGroup(uid_t uid) const
        return mapGroups[type_list::IDX<T, MatchItemTypes>::idx][uid];
 }
 
-void NaivePolicyDb::clear() {
-       backend.clear();
+bool NaivePolicyDb::init(const char *filename) {
+       return backend.init(filename);
 }
 
 template <typename T>
@@ -164,7 +156,6 @@ const std::map<gid_t, T> &NaivePolicyDb::getPoliciesGroup() const {
  */
 #define T_INSTANTIATION(T) \
        template const VGid *NaivePolicyDb::getGroups<MatchItem##T>(uid_t, gid_t) const; \
-       template void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, Item##T &item); \
        template const Policy##T &NaivePolicyDb::getPolicyContextMandatory() const; \
        template const Policy##T &NaivePolicyDb::getPolicyContextDefault() const;
 

diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp
index 9137dfc..1bbea87 100644 (file)
--- a/src/internal/naive_policy_db.hpp
+++ b/src/internal/naive_policy_db.hpp
@@ -86,7 +86,7 @@ namespace ldp_xml_parser
                                         const PolicyTypeValue policy_type_value,
                                         T &item);
 
-               void clear();
+               bool init(const char *filename);
 
                // The getPolicy*() methods are intended for Serializers to get
                // access to policy structures.

diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp
index 15ef3c3..6863b3e 100644 (file)
--- a/src/internal/policy.cpp
+++ b/src/internal/policy.cpp
@@ -316,8 +316,8 @@ ItemAccess *ItemBuilder::getAccessItem() {
        return &__current_access;
 }
 
-ItemBuilder::ItemBuilder()
-       : __current_item_type(ItemType::GENERIC)
+ItemBuilder::ItemBuilder(ldp_xml::StorageBackendXML &storage)
+       : __storage(storage), __current_item_type(ItemType::GENERIC)
 {
        reset();
 }
@@ -331,24 +331,23 @@ void ItemBuilder::reset() {
        __current_item_type = ItemType::GENERIC;
 }
 
-void ItemBuilder::generateItem(BusType bus_type, PolicyType& policy_type, PolicyTypeValue& policy_type_value) {
-       NaivePolicyDb &db = policy_checker().getPolicyDb(bus_type);
+void ItemBuilder::generateItem(PolicyType& policy_type, PolicyTypeValue& policy_type_value) {
        switch (__current_item_type) {
                case ItemType::OWN:
                        __current_own.__decision = __decision;
-                       db.addItem(policy_type, policy_type_value, __current_own);
+                       __storage.addItem(policy_type, policy_type_value, __current_own);
                        break;
                case ItemType::SEND:
                        __current_send.__decision = __decision;
-                       db.addItem(policy_type, policy_type_value, __current_send);
+                       __storage.addItem(policy_type, policy_type_value, __current_send);
                        break;
                case ItemType::RECEIVE:
                        __current_receive.__decision = __decision;
-                       db.addItem(policy_type, policy_type_value, __current_receive);
+                       __storage.addItem(policy_type, policy_type_value, __current_receive);
                        break;
                case ItemType::ACCESS:
                        __current_access.__decision = __decision;
-                       db.addItem(policy_type, policy_type_value, __current_access);
+                       __storage.addItem(policy_type, policy_type_value, __current_access);
                        break;
                case ItemType::GENERIC:
                        // do nothing

diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp
index c0d1d9b..1ef7ed5 100644 (file)
--- a/src/internal/policy.hpp
+++ b/src/internal/policy.hpp
@@ -34,6 +34,10 @@
 /** Maximum tree node children. It is connected with proper characters which can be used in name.*/
 #define MAX_CHILDREN 65
 
+namespace ldp_xml {
+       class StorageBackendXML;
+}
+
 namespace ldp_xml_parser
 {
        /** Type of sent message*/
@@ -246,11 +250,11 @@ namespace ldp_xml_parser
 
        using MatchItemTypes = type_list::type_list<MatchItemOwn, MatchItemSend, MatchItemReceive, MatchItemAccess>;
 
-       class NaivePolicyDb;
-
        /** Allows to create items from given name, decision etc. */
        class ItemBuilder {
        private:
+               ldp_xml::StorageBackendXML &__storage;
+
                DecisionItem __decision;
                ItemType __current_item_type;
 
@@ -263,9 +267,9 @@ namespace ldp_xml_parser
                ItemSendReceive* getSendReceiveItem();
                ItemAccess* getAccessItem();
        public:
-               ItemBuilder();
+               explicit ItemBuilder(ldp_xml::StorageBackendXML &storage);
                ~ItemBuilder();
-               void generateItem(BusType bus_type, PolicyType& policy_type, PolicyTypeValue& policy_type_value);
+               void generateItem(PolicyType& policy_type, PolicyTypeValue& policy_type_value);
                void reset();
                void setOwnItem();
                void setSendItem();

diff --git a/src/internal/storage_backend_xml.cpp b/src/internal/storage_backend_xml.cpp
index 5cfb9cb..4247108 100644 (file)
--- a/src/internal/storage_backend_xml.cpp
+++ b/src/internal/storage_backend_xml.cpp
@@ -1,5 +1,6 @@
 #include "storage_backend_xml.hpp"
 #include "policy_containers.hpp"
+#include "xml_parser.hpp"
 
 using namespace ldp_xml_parser;
 
@@ -264,11 +265,14 @@ template <typename T>
 void StorageBackendXML::addItem(const ldp_xml_parser::PolicyType policy_type,
                                                                const ldp_xml_parser::PolicyTypeValue policy_type_value,
                                                                T &item) {
+       tslog::log("Add item: ", item, ", decision: ", item.getDecision(), "\n");
        pimpl->getPolicySet<typename T::match_type>().addItem(policy_type, policy_type_value, item);
 }
 
-void StorageBackendXML::clear() {
+bool StorageBackendXML::init(const char *filename) {
        pimpl.reset(new StorageBackendXMLImpl);
+       XmlParser parser(*this, filename);
+       return parser.parsePolicyConfigFile() == 0;
 }
 
 void StorageBackendXML::printContent() const {

diff --git a/src/internal/storage_backend_xml.hpp b/src/internal/storage_backend_xml.hpp
index dd4cffc..84b2acb 100644 (file)
--- a/src/internal/storage_backend_xml.hpp
+++ b/src/internal/storage_backend_xml.hpp
@@ -20,7 +20,7 @@ public:
                                 const ldp_xml_parser::PolicyTypeValue policy_type_value,
                                 T &item);
 
-       void clear();
+       bool init(const char *filename);
 
        void printContent() const;
 

diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp
index 2433e03..0835c37 100644 (file)
--- a/src/internal/xml_parser.cpp
+++ b/src/internal/xml_parser.cpp
@@ -47,8 +47,7 @@ public:
 }
 
 void start_element_handler(void *data, const char *el, const char **attr) {
-       (void)data;
-       XmlParser& parser = static_parser();
+       XmlParser& parser = *reinterpret_cast<XmlParser*>(data);
        try {
                parser.elementStart(el, attr);
        } catch (...) {
@@ -58,8 +57,7 @@ void start_element_handler(void *data, const char *el, const char **attr) {
 }
 
 void end_element_handler(void *data, const char *el) {
-       (void)data;
-       XmlParser& parser = static_parser();
+       XmlParser& parser = *reinterpret_cast<XmlParser*>(data);
        try {
                parser.elementEnd(el);
        } catch (...) {
@@ -69,8 +67,7 @@ void end_element_handler(void *data, const char *el) {
 }
 
 void text_handler(void *data, const char *text, int len) {
-       (void)data;
-       XmlParser& parser = static_parser();
+       XmlParser& parser = *reinterpret_cast<XmlParser*>(data);
        try {
                parser.text(text, len);
        } catch (...) {
@@ -80,13 +77,7 @@ void text_handler(void *data, const char *text, int len) {
 }
 
 bool XmlParser::isMainConfFile(const char *filename) {
-       switch (curr_bus) {
-       case SYSTEM_BUS:
-               return strcmp(filename, SYSTEM_BUS_CONF_FILE_PRIMARY) == 0 || strcmp(filename, SYSTEM_BUS_CONF_FILE_SECONDARY) == 0;
-       case SESSION_BUS:
-               return strcmp(filename, SESSION_BUS_CONF_FILE_PRIMARY) == 0 || strcmp(filename, SESSION_BUS_CONF_FILE_SECONDARY) == 0;
-       }
-       return false;
+       return strcmp(main_filename, filename) == 0;
 }
 
 void XmlParser::setRetCode(int ret) {
@@ -176,10 +167,9 @@ void XmlParser::elementEnd(const char *el) {
        }
 }
 
-int XmlParser::parsePolicyConfigFile(BusType bus, const char *fname) {
-       tslog::log("XmlParser::parsePolicyConfigFile called with filename: ", fname, "\n");
-       curr_bus = bus;
-       parsePolicyConfigFileInternal(fname);
+int XmlParser::parsePolicyConfigFile() {
+       tslog::log("XmlParser::parsePolicyConfigFile called with filename: ", main_filename, "\n");
+       parsePolicyConfigFileInternal(main_filename);
        return ret_code;
 }
 
@@ -246,6 +236,7 @@ void XmlParser::parseXmlFile(const char *filename) {
 
        auto parser = std::unique_ptr<XML_ParserStruct, std::function<void(XML_Parser)>>(XML_ParserCreate(nullptr),
                [](XML_Parser p) { XML_ParserFree(p); });
+       XML_SetUserData(parser.get(), this);
        XML_SetElementHandler(parser.get(), start_element_handler, end_element_handler);
        XML_SetCharacterDataHandler(parser.get(), text_handler);
 
@@ -371,7 +362,7 @@ void XmlParser::parseRule(Decision decision, const char **attr)
                parseRuleAttribute(attr[i], attr[i + 1]);
        }
 
-       __builder.generateItem(curr_bus, policy_type, policy_type_value);
+       __builder.generateItem(policy_type, policy_type_value);
 }
 
 namespace {
@@ -443,4 +434,6 @@ void XmlParser::parseRuleAttribute(const char *name, const char *value)
                __builder.addMessageType(__str_to_message_type(value));
 }
 
-DEF_NODESTRUCT_GLOBAL(ldp_xml_parser::XmlParser, static_parser);
+XmlParser::XmlParser(ldp_xml::StorageBackendXML &storage, const char *fname)
+       : __builder{storage}, main_filename{fname}
+{}

diff --git a/src/internal/xml_parser.hpp b/src/internal/xml_parser.hpp
index a47b963..12afd0c 100644 (file)
--- a/src/internal/xml_parser.hpp
+++ b/src/internal/xml_parser.hpp
@@ -25,6 +25,11 @@
 #include <boost/noncopyable.hpp>
 #include "policy.hpp"
 #include "global_nodestruct.hpp"
+#include <boost/utility/string_ref.hpp>
+
+namespace ldp_xml {
+       class StorageBackendXML;
+}
 
 namespace ldp_xml_parser
 {
@@ -33,7 +38,7 @@ namespace ldp_xml_parser
        {
        public:
                /** Parses given config file for declared bus type */
-               int parsePolicyConfigFile(BusType bus_type, const char *fname);
+               int parsePolicyConfigFile();
 
                void elementStart(const char *el, const char **attr);
 
@@ -43,6 +48,8 @@ namespace ldp_xml_parser
 
                void setRetCode(int ret);
 
+               explicit XmlParser(ldp_xml::StorageBackendXML &storage, const char *fname);
+
        private:
                class IncludeItem;
 
@@ -73,20 +80,18 @@ namespace ldp_xml_parser
 
                typedef enum { IDLE, BUSCONFIG, INCLUDE, INCLUDEDIR, POLICY, RULE } ParseState;
 
-               ParseState state;
+               ParseState state{IDLE};
 
                bool ignore_always;
                bool ignore_missing;
-               BusType curr_bus;
                std::string current_text;
                std::string curr_dir;
                int ret_code;
+               const char *main_filename;
 
                std::vector<std::string> included_files;
        };
 
 } //namespace
 
-DCL_NODESTRUCT_GLOBAL(ldp_xml_parser::XmlParser, static_parser)
-
 #endif