more refactoring

4 files changed, 120 insertions, 133 deletions

diff --git a/worker/ipaaction.c b/worker/ipaaction.c
index b7ef871..7bf7422 100644
--- a/worker/ipaaction.c
+++ b/worker/ipaaction.c
@@ -22,6 +22,36 @@
 #include "helpers.h"
 #include "xml_helper.h"
 
+#define XPATH_IPAACTION_CONDITION_COMMAND (xmlChar *) "//def:ipa/def:ipaaction/def:condition/def:command"
+#define XPATH_IPAACTION_CONDITION_USER (xmlChar *) "//def:ipa/def:ipaaction/def:condition/def:user"
+#define XPATH_IPAACTION_CONDITION_GROUP (xmlChar *) "//def:ipa/def:ipaaction/def:condition/def:group"
+
+#define XPATH_IPAACTION_FILE_URL (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:url"
+#define XPATH_IPAACTION_FILE_DATA (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:data"
+#define XPATH_IPAACTION_FILE_PATH (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:path"
+#define XPATH_IPAACTION_FILE_OWNER (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:owner"
+#define XPATH_IPAACTION_FILE_GROUP (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:group"
+#define XPATH_IPAACTION_FILE_ACCESS (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:access"
+#define XPATH_IPAACTION_FILE_SELINUX_CONTEXT (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:selinux_context"
+#define XPATH_IPAACTION_FILE_CLEANUP (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:cleanup"
+
+#define XPATH_IPAACTION_RUN_COMMAND (xmlChar *) "//def:ipa/def:ipaaction/def:run/def:command"
+#define XPATH_IPAACTION_RUN_USER (xmlChar *) "//def:ipa/def:ipaaction/def:run/def:user"
+#define XPATH_IPAACTION_RUN_GROUP (xmlChar *) "//def:ipa/def:ipaaction/def:run/def:group"
+
+char *find_value(const xmlDocPtr doc, const xmlChar *xpath_expr, const char *default_value, const xmlChar *prefix, const xmlChar *namespace) {
+    char *val;
+    val = find_by_xpath(doc, xpath_expr, FIND_VALUE, prefix, namespace);
+    if (val==NULL) {
+        DEBUG(3, ("No value found with XPath %s.\n", xpath_expr));
+        if (default_value!=NULL) {
+            DEBUG(3, ("Using default value %s.\n", default_value));
+            val=strdup(default_value);
+        }
+    }
+
+    return val;
+}
 
 int check_ipaaction_condition(const xmlDocPtr doc, const xmlChar *default_namespace) {
     int ret;
@@ -30,31 +60,17 @@ int check_ipaaction_condition(const xmlDocPtr doc, const xmlChar *default_namesp
     char *group;
     char *arguments;
 
-    condition = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:condition/def:command",
+    condition = find_value(doc, XPATH_IPAACTION_CONDITION_COMMAND, NULL,
         default_namespace_prefix, default_namespace);
-    if ( condition==NULL ) {
-        DEBUG(3, ("No condition found for current ipaaction.\n"));
-        return 0;
-    }
+    CHECK(condition, NULL, ("No condition found for current ipaaction.\n"), return 0);
     DEBUG(3, ("Found condition for current ipaaction: |%s|\n", condition));
 
-    user = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:condition/def:user",
+    user = find_value(doc, XPATH_IPAACTION_CONDITION_USER, "nobody",
         default_namespace_prefix, default_namespace);
-    if (user==NULL) {
-        DEBUG(3, ("User for condition not found, using default"));
-        user=strdup("nobody");
-    }
     DEBUG(3, ("Found user for condition: %s\n", user));
 
-    group = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:condition/def:group",
+    group = find_value(doc, XPATH_IPAACTION_CONDITION_GROUP, "nobody",
         default_namespace_prefix, default_namespace);
-    if (group==NULL) {
-        DEBUG(3, ("Group for condition not found, using default\n"));
-        group=strdup("nobody");
-    }
     DEBUG(3, ("Found group for condition: %s\n", group));
 
     arguments=strchr(condition,' ');
@@ -90,12 +106,10 @@ int ipaaction_file(const xmlDocPtr doc, const xmlChar *default_namespace) {
     int ret;
     struct stat stat_buffer;
 
-    url = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:url",
+    url = find_value(doc, XPATH_IPAACTION_FILE_URL, NULL,
         default_namespace_prefix, default_namespace);
     DEBUG(3, ("Found the following ipaaction file url: |%s|\n", url));
-    data = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:data",
+    data = find_value(doc, XPATH_IPAACTION_FILE_DATA, NULL,
         default_namespace_prefix, default_namespace);
     DEBUG(3, ("Found the following ipaaction file data: |%s|\n", data));
     if (url==NULL && data==NULL) {
@@ -107,57 +121,31 @@ int ipaaction_file(const xmlDocPtr doc, const xmlChar *default_namespace) {
         return -1;
     }
 
-    path = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:path",
+    path = find_value(doc, XPATH_IPAACTION_FILE_PATH, NULL,
         default_namespace_prefix, default_namespace);
     CHECK(path, NULL, ("Path for ipaaction file not found.\n"), return -1);
     DEBUG(3, ("Found path for ipaaction file: %s\n", path));
     ret=stat(path, &stat_buffer);
     CHECK(ret, 0, ("Destination file %s alread exists.\n", path), return -1); 
 
-    owner = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:owner",
+    owner = find_value(doc, XPATH_IPAACTION_FILE_OWNER, "root",
         default_namespace_prefix, default_namespace);
-    if (owner==NULL) {
-        DEBUG(3, ("Owner for ipaaction file not found, using default\n"));
-        owner=strdup("root");
-    }
     DEBUG(3, ("Found owner for ipaaction file: %s\n", owner));
 
-    group = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:group",
+    group = find_value(doc, XPATH_IPAACTION_FILE_GROUP, "root",
         default_namespace_prefix, default_namespace);
-    if (group==NULL) {
-        DEBUG(3, ("Group for ipaaction file not found, using default\n"));
-        group=strdup("root");
-    }
     DEBUG(3, ("Found group for ipaaction file: %s\n", group));
 
-    access = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:access",
+    access = find_value(doc, XPATH_IPAACTION_FILE_ACCESS, "0400",
         default_namespace_prefix, default_namespace);
-    if (access==NULL) {
-        DEBUG(3, ("Access permissions for ipaaction file not found, using default\n"));
-        group=strdup("0400");
-    }
     DEBUG(3, ("Found access permissions for ipaaction file: %s\n", access));
 
-    selinux_context = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:selinux_context",
+    selinux_context = find_value(doc, XPATH_IPAACTION_FILE_SELINUX_CONTEXT, NULL,
         default_namespace_prefix, default_namespace);
-    if (selinux_context==NULL) {
-        DEBUG(3, ("SELinux file context for ipaaction file not found, using none\n"));
-        selinux_context=NULL;
-    }
     DEBUG(3, ("Found SELinux file context for ipaaction file: %s\n", selinux_context));
 
-    cleanup = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:file/def:cleanup",
+    cleanup = find_value(doc, XPATH_IPAACTION_FILE_CLEANUP, "no",
         default_namespace_prefix, default_namespace);
-    if (cleanup==NULL) {
-        DEBUG(3, ("No cleanup information for ipaaction file not found, assuming no\n"));
-        cleanup=strdup("no");
-    }
     DEBUG(3, ("Found cleanup information for ipaaction file: %s\n", cleanup));
 
 
@@ -198,30 +186,19 @@ int ipaaction_run(const xmlDocPtr doc, const xmlChar *default_namespace) {
     char *group;
     char *arguments;
 
-    command = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:run/def:command",
+    command = find_value(doc, XPATH_IPAACTION_RUN_COMMAND, NULL,
         default_namespace_prefix, default_namespace);
     CHECK(command, NULL,
         ("No command in ipaaction run section found, this should neven happen.\n"),
         return -1);
     DEBUG(3, ("Found command for current ipaaction: |%s|\n", command));
 
-    user = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:run/def:user",
+    user = find_value(doc, XPATH_IPAACTION_RUN_USER, "nobody",
         default_namespace_prefix, default_namespace);
-    if (user==NULL) {
-        DEBUG(3, ("User for ipaaction run command not found, using default"));
-        user=strdup("nobody");
-    }
     DEBUG(3, ("Found user for ipaaction run command: %s\n", user));
 
-    group = find_value_by_xpath(doc,
-        (xmlChar *) "//def:ipa/def:ipaaction/def:run/def:group",
+    group = find_value(doc, XPATH_IPAACTION_RUN_GROUP, "nobody",
         default_namespace_prefix, default_namespace);
-    if (group==NULL) {
-        DEBUG(3, ("Group for ipaaction run command not found, using default\n"));
-        group=strdup("nobody");
-    }
     DEBUG(3, ("Found group for ipaaction run command: %s\n", group));
 
     arguments=strchr(command,' ');
diff --git a/worker/output_handler.c b/worker/output_handler.c
index 728f53b..ca55f07 100644
--- a/worker/output_handler.c
+++ b/worker/output_handler.c
@@ -101,7 +101,8 @@ int output_handler_file(xmlNode *node, const xmlDocPtr doc, const char *xslt_fil
     CHECK_NULL_RETURN(tmp_file_name,("malloc failed."));
     strcpy(tmp_file_name, name);
     strcat(tmp_file_name, ".XXXXXX");
-    open_temporary_file(tmp_file_name, permission, owner, group, NULL);
+    fd=open_temporary_file(tmp_file_name, permission, owner, group, NULL);
+    CHECK(fd, -1, ("Failed to open temporary file.\n"), return -1);
  
     parsed_stylesheet = xsltParseStylesheetFile((xmlChar *) xslt_file_name);
     CHECK_NULL_FATAL(parsed_stylesheet, ("Cannot parse stylesheet!\n"));
diff --git a/worker/xml_helper.c b/worker/xml_helper.c
index 34a1514..6d91e13 100644
--- a/worker/xml_helper.c
+++ b/worker/xml_helper.c
@@ -25,6 +25,26 @@
  */
 xmlChar *default_namespace_prefix = (xmlChar *) "def";
 
+/** 
+ * \brief Validate a XML policy file
+ *
+ * Call this function bedore any further processing of a XML policy file. It
+ * will extract the name of the RELAX NG schema file from the metadata section
+ * together with other information and validate the file accordingly.
+ *
+ * \param policy_file_name name of the XML policy file
+ * \param default_namespace will contain the default namespace of the XML
+ * policy file if the function returns successfully
+ * \param ipa_policy_type will contain the IPA policy type, i.e. action,
+ * config or role, if the function returns successfully
+ * \param xslt_file_name will contain the name of the XSLT file if the IPA
+ * policy is either config or role (action do not need an XSLT) and if the
+ * function returns successfully
+ *
+ * \return 0 on success, -1 if an error occured
+ *
+ */
+
 int validate_policy(const char *policy_file_name, xmlChar **default_namespace, char **ipa_policy_type, char **xslt_file_name) {
     xmlDocPtr doc;
     xmlNodePtr root_node;
@@ -54,7 +74,7 @@ int validate_policy(const char *policy_file_name, xmlChar **default_namespace, c
 
     xmlStrPrintf(xpath_expr, XMLCHARLEN, (xmlChar *) "//%s:ipa/*[2]",
                  default_namespace_prefix);
-    *ipa_policy_type = find_name_by_xpath(doc, xpath_expr, default_namespace_prefix, *default_namespace);
+    *ipa_policy_type = find_by_xpath(doc, xpath_expr, FIND_NAME, default_namespace_prefix, *default_namespace);
     CHECK(*ipa_policy_type, NULL, ("Type of IPA policy not found.\n"), exit(1));
     DEBUG(3, ("Found IPA policy type: %s\n", *ipa_policy_type));
     if ( strncmp(*ipa_policy_type, "ipaconfig",9) != 0 &&
@@ -67,7 +87,7 @@ int validate_policy(const char *policy_file_name, xmlChar **default_namespace, c
     xmlStrPrintf(xpath_expr, XMLCHARLEN, (xmlChar *) "//%s:RNGfile",
                  default_namespace_prefix);
     rng_file_name =
-        find_value_by_xpath(doc, xpath_expr, default_namespace_prefix,
+        find_by_xpath(doc, xpath_expr, FIND_VALUE, default_namespace_prefix,
                             *default_namespace);
     CHECK(rng_file_name, NULL, ("Name of RELANX NG schema file not found.\n"), exit(1));
     DEBUG(3, ("Found name of RELAX NG schema file: %s\n", rng_file_name));
@@ -80,7 +100,7 @@ int validate_policy(const char *policy_file_name, xmlChar **default_namespace, c
                                (xmlRelaxNGNewParserCtxt(rng_file_name)));
     CHECK(rng_context, NULL, ("Failed to create RNG context\n"), exit(1));
     if (xmlRelaxNGValidateDoc(rng_context, doc) == 0) {
-        DEBUG(0, ("The document is valid.\n"));
+        DEBUG(3, ("The document is valid.\n"));
     } else {
         DEBUG(0, ("Error during validation.\n"));
         exit(1);
@@ -93,7 +113,7 @@ int validate_policy(const char *policy_file_name, xmlChar **default_namespace, c
     if (strncmp(*ipa_policy_type, "ipaaction", 9)!=0) {
         xmlStrPrintf(xpath_expr, XMLCHARLEN, (xmlChar *) "//%s:XSLTfile", default_namespace_prefix);
         *xslt_file_name =
-            find_value_by_xpath(doc, xpath_expr, default_namespace_prefix, *default_namespace);
+            find_by_xpath(doc, xpath_expr, FIND_VALUE, default_namespace_prefix, *default_namespace);
         CHECK(*xslt_file_name, NULL, ("Name of XSLT file not found.\n"), exit(1));
         DEBUG(3, ("Found name of XSLT file: %s\n", *xslt_file_name));
     }
@@ -103,7 +123,18 @@ int validate_policy(const char *policy_file_name, xmlChar **default_namespace, c
     return 0;
 }
 
-int print_all_attributes(xmlNode *node) {
+/**
+ * \brief Show all attributes of an XML node
+ *
+ * This is a debung function to print all attribute of a XML node.
+ *
+ * \param node pointer to the XML node
+ *
+ * \return 0
+ *
+ */
+
+int print_all_attributes(const xmlNode *node) {
     xmlAttr *cur;
 
     cur=node->properties;
@@ -114,58 +145,24 @@ int print_all_attributes(xmlNode *node) {
     return 0;
 }
 
-char *find_name_by_xpath(xmlDocPtr doc, xmlChar * xpath_expr, xmlChar * prefix,
-                          xmlChar * namespace)
-{
-
-    xmlXPathContextPtr xpath_context;
-    xmlXPathObjectPtr xpath_obj;
-    char *result = NULL;
-
-    /* Create xpath evaluation context */
-    xpath_context = xmlXPathNewContext(doc);
-    CHECK_NULL_FATAL(xpath_context,
-                     ("Error: unable to create new XPath context\n"));
-    /* Register a namespace */
-    if (xmlXPathRegisterNs(xpath_context, prefix, namespace) != 0) {
-        DEBUG(0,
-              ("Error: unable to register NS with prefix=\"%s\" and href=\"%s\"\n",
-               prefix , namespace));
-        xmlXPathFreeContext(xpath_context);
-        return NULL;
-    }
-    /* Evaluate xpath expression */
-    xpath_obj = xmlXPathEvalExpression(xpath_expr, xpath_context);
-    if (xpath_obj == NULL) {
-        DEBUG(0,
-              ("Error: unable to evaluate xpath expression \"%s\"\n",
-               xpath_expr));
-        xmlXPathFreeContext(xpath_context);
-        return NULL;
-    }
-
-    if (xmlXPathNodeSetIsEmpty(xpath_obj->nodesetval)) {
-        DEBUG(0, ("Nothing found for %s\n", xpath_expr));
-        xmlXPathFreeObject(xpath_obj);
-        xmlXPathFreeContext(xpath_context);
-        return NULL;
-    } else if (xmlXPathNodeSetGetLength(xpath_obj->nodesetval) != 1) {
-        DEBUG(0, ("More than one node found for %s!", xpath_expr));
-        xmlXPathFreeObject(xpath_obj);
-        xmlXPathFreeContext(xpath_context);
-        return NULL;
-    } else {
-        result = strdup((char *) xpath_obj->nodesetval->nodeTab[0]->name);
-    }
-
-
-    xmlXPathFreeObject(xpath_obj);
-    xmlXPathFreeContext(xpath_context);
-    return result;
-
-}
-
-char *find_value_by_xpath(const xmlDocPtr doc, const xmlChar * xpath_expr, const xmlChar * prefix,
+/**
+ * \brief find a single name or value defined by a XPath expression
+ *
+ * This function will return the name or the value of a XML node which is
+ * selected by a XPath expression. if more than one value is found it is
+ * considered as an error and NULL is returned.
+ *
+ * \param doc pointer to the already parsed XML document
+ * \param xpath_expr a XPath expression describing the node to search for
+ * \param type use FIND_NAME to return the name and FIND_VALUE to return the
+ * value of the node
+ * \param prefix prefix of the namespace of the node to search for
+ * \param namespare namespace URI of the node to search for
+ *
+ * \return pointer to the found string or NULL in case of an error
+ *
+ */
+char *find_by_xpath(const xmlDocPtr doc, const xmlChar * xpath_expr, const int type, const xmlChar * prefix,
                           const xmlChar * namespace)
 {
 
@@ -206,10 +203,19 @@ char *find_value_by_xpath(const xmlDocPtr doc, const xmlChar * xpath_expr, const
         xmlXPathFreeContext(xpath_context);
         return NULL;
     } else {
-        result =
-            (char *) xmlNodeListGetString(doc,
+        switch (type) {
+            case FIND_NAME:
+                result = strdup((char *) xpath_obj->nodesetval->nodeTab[0]->name);
+                break;
+            case FIND_VALUE:
+                result =
+                    (char *) xmlNodeListGetString(doc,
                                           xpath_obj->nodesetval->nodeTab[0]->
                                           xmlChildrenNode, 1);
+                break;
+            default:
+                DEBUG(0,("Unknow search type %d\n"));
+        }
     }
 
 
diff --git a/worker/xml_helper.h b/worker/xml_helper.h
index 7cd7dc3..a2b7705 100644
--- a/worker/xml_helper.h
+++ b/worker/xml_helper.h
@@ -9,13 +9,16 @@
 #define XSLT_METADATA_NAMESPACE_PREFIX (xmlChar *) "md"
 #define XPATH_OUTPUT_HANDLER (xmlChar *) "//md:output_handler/md:*"
 
+enum {
+    FIND_NAME = 1,
+    FIND_VALUE
+};
+
 extern xmlChar *default_namespace_prefix;
 
 int validate_policy(const char *policy_file_name, xmlChar **default_namespace, char **ipa_policy_type, char **xslt_file_name);
 
-int print_all_attributes(xmlNode *node);
+int print_all_attributes(const xmlNode *node);
 
-char *find_name_by_xpath(xmlDocPtr doc, xmlChar * xpath_expr, xmlChar * prefix,
-                          xmlChar * namespace);
-char *find_value_by_xpath(const xmlDocPtr doc, const xmlChar * xpath_expr, const xmlChar * prefix,
-                          const xmlChar * namespace);
+char *find_by_xpath(const xmlDocPtr doc, const xmlChar * xpath_expr,
+    const int type, const xmlChar * prefix, const xmlChar * namespace);