File bug-1024037_pacemaker-libcrmcommon-delete-XML-c... of Package pacemaker.8397

Overview Repositories Revisions Requests Users Attributes Meta

File bug-1024037_pacemaker-libcrmcommon-delete-XML-comments.patch of Package pacemaker.8397

commit 362f02874b6be46bf2648fb45ebc6bc636d48965
Author: Gao,Yan <ygao@suse.com>
Date:   Mon Feb 13 17:47:23 2017 +0100

Fix: libcrmcommon: Correctly delete XML comments according to their positions
    
    Previously, v2 patchset was not able to define which exact XML comments were
    expected to be deleted in a CIB XML element. It was a problem if there
    was more than one comment at the same level within a CIB XML element.
    
    This commit resolves it by adding and handling a "position" field in the
    diff operation "delete" for XML comments.

diff --git a/lib/common/xml.c b/lib/common/xml.c
index fd80fe189..6dce4cb7f 100644
--- a/lib/common/xml.c
+++ b/lib/common/xml.c
@@ -78,7 +78,7 @@ typedef struct xml_private_s
         uint32_t flags;
         char *user;
         GListPtr acls;
-        GListPtr deleted_paths;
+        GListPtr deleted_objs;
 } xml_private_t;
 
 typedef struct xml_acl_s {
@@ -86,6 +86,11 @@ typedef struct xml_acl_s {
         char *xpath;
 } xml_acl_t;
 
+typedef struct xml_deleted_obj_s {
+        char *path;
+        int position;
+} xml_deleted_obj_t;
+
 /* *INDENT-OFF* */
 
 static filter_t filter[] = {
@@ -275,6 +280,17 @@ __xml_acl_free(void *data)
 }
 
 static void
+__xml_deleted_obj_free(void *data)
+{
+    if(data) {
+        xml_deleted_obj_t *deleted_obj = data;
+
+        free(deleted_obj->path);
+        free(deleted_obj);
+    }
+}
+
+static void
 __xml_private_clean(xml_private_t *p)
 {
     if(p) {
@@ -288,9 +304,9 @@ __xml_private_clean(xml_private_t *p)
             p->acls = NULL;
         }
 
-        if(p->deleted_paths) {
-            g_list_free_full(p->deleted_paths, free);
-            p->deleted_paths = NULL;
+        if(p->deleted_objs) {
+            g_list_free_full(p->deleted_objs, __xml_deleted_obj_free);
+            p->deleted_objs = NULL;
         }
     }
 }
@@ -1094,10 +1110,10 @@ is_config_change(xmlNode *xml)
 
     if(xml->doc && xml->doc->_private) {
         p = xml->doc->_private;
-        for(gIter = p->deleted_paths; gIter; gIter = gIter->next) {
-            char *path = gIter->data;
+        for(gIter = p->deleted_objs; gIter; gIter = gIter->next) {
+            xml_deleted_obj_t *deleted_obj = gIter->data;
 
-            if(strstr(path, "/"XML_TAG_CIB"/"XML_CIB_TAG_CONFIGURATION) != NULL) {
+            if(strstr(deleted_obj->path, "/"XML_TAG_CIB"/"XML_CIB_TAG_CONFIGURATION) != NULL) {
                 return TRUE;
             }
         }
@@ -1241,11 +1257,15 @@ xml_create_patchset_v2(xmlNode *source, xmlNode *target)
         crm_xml_add(v, vfields[lpc], value);
     }
 
-    for(gIter = doc->deleted_paths; gIter; gIter = gIter->next) {
+    for(gIter = doc->deleted_objs; gIter; gIter = gIter->next) {
+        xml_deleted_obj_t *deleted_obj = gIter->data;
         xmlNode *change = create_xml_node(patchset, XML_DIFF_CHANGE);
 
         crm_xml_add(change, XML_DIFF_OP, "delete");
-        crm_xml_add(change, XML_DIFF_PATH, gIter->data);
+        crm_xml_add(change, XML_DIFF_PATH, deleted_obj->path);
+        if (deleted_obj->position >= 0) {
+            crm_xml_add_int(change, XML_DIFF_POSITION, deleted_obj->position);
+        }
     }
 
     __xml_build_changes(target, patchset);
@@ -1473,7 +1493,15 @@ xml_log_patchset(uint8_t log_level, const char *function, xmlNode * patchset)
                 }
 
             } else if(strcmp(op, "delete") == 0) {
-                do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s", xpath);
+                int position = -1;
+
+                crm_element_value_int(change, XML_DIFF_POSITION, &position);
+                if (position >= 0) {
+                    do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s (%d)", xpath, position);
+
+                } else {
+                    do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s", xpath);
+                }
             }
         }
         return;
@@ -1521,8 +1549,17 @@ xml_log_changes(uint8_t log_level, const char *function, xmlNode * xml)
         return;
     }
 
-    for(gIter = doc->deleted_paths; gIter; gIter = gIter->next) {
-        do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s", (char*)gIter->data);
+    for(gIter = doc->deleted_objs; gIter; gIter = gIter->next) {
+        xml_deleted_obj_t *deleted_obj = gIter->data;
+
+        if (deleted_obj->position >= 0) {
+            do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s (%d)",
+                             deleted_obj->path, deleted_obj->position);
+
+        } else {
+            do_crm_log_alias(log_level, __FILE__, function, __LINE__, "-- %s",
+                             deleted_obj->path);
+        }
     }
 
     log_data_element(log_level, __FILE__, function, __LINE__, "+ ", xml, 0,
@@ -1881,7 +1918,7 @@ xml_apply_patchset_v1(xmlNode *xml, xmlNode *patchset, bool check_version)
 }
 
 static xmlNode *
-__first_xml_child_match(xmlNode *parent, const char *name, const char *id)
+__first_xml_child_match(xmlNode *parent, const char *name, const char *id, int position)
 {
     xmlNode *cIter = NULL;
 
@@ -1894,13 +1931,21 @@ __first_xml_child_match(xmlNode *parent, const char *name, const char *id)
                 continue;
             }
         }
+
+        /* The "position" makes sense only for XML comments for now */
+        if (cIter->type == XML_COMMENT_NODE
+            && position >= 0
+            && __xml_offset(cIter) != position) {
+            continue;
+        }
+
         return cIter;
     }
     return NULL;
 }
 
 static xmlNode *
-__xml_find_path(xmlNode *top, const char *key)
+__xml_find_path(xmlNode *top, const char *key, int target_position)
 {
     xmlNode *target = (xmlNode*)top->doc;
     char *id = malloc(XML_BUFFER_SIZE);
@@ -1923,13 +1968,19 @@ __xml_find_path(xmlNode *top, const char *key)
 
         } else if(tag && section) {
             int f = sscanf (section, "%[^[][@id='%[^']", tag, id);
+            int current_position = -1;
+
+            /* The "target_position" is for the target tag */
+            if (rc == 1 && target_position >= 0) {
+                current_position = target_position;
+            }
 
             switch(f) {
                 case 1:
-                    target = __first_xml_child_match(target, tag, NULL);
+                    target = __first_xml_child_match(target, tag, NULL, current_position);
                     break;
                 case 2:
-                    target = __first_xml_child_match(target, tag, id);
+                    target = __first_xml_child_match(target, tag, id, current_position);
                     break;
                 default:
                     crm_trace("Aborting on %s", section);
@@ -1975,16 +2026,20 @@ xml_apply_patchset_v2(xmlNode *xml, xmlNode *patchset, bool check_version)
         xmlNode *match = NULL;
         const char *op = crm_element_value(change, XML_DIFF_OP);
         const char *xpath = crm_element_value(change, XML_DIFF_PATH);
+        int position = -1;
 
         crm_trace("Processing %s %s", change->name, op);
         if(op == NULL) {
             continue;
         }
 
+        if(strcmp(op, "delete") == 0) {
+            crm_element_value_int(change, XML_DIFF_POSITION, &position);
+        }
 #if 0
         match = get_xpath_object(xpath, xml, LOG_TRACE);
 #else
-        match = __xml_find_path(xml, xpath);
+        match = __xml_find_path(xml, xpath, position);
 #endif
         crm_trace("Performing %s on %s with %p", op, xpath, match);
 
@@ -2583,8 +2638,8 @@ xml_get_path(xmlNode *xml)
     return NULL;
 }
 
-void
-free_xml(xmlNode * child)
+static void
+free_xml_with_position(xmlNode * child, int position)
 {
     if (child != NULL) {
         xmlNode *top = NULL;
@@ -2613,9 +2668,25 @@ free_xml(xmlNode * child)
                 char buffer[XML_BUFFER_SIZE];
 
                 if(__get_prefix(NULL, child, buffer, offset) > 0) {
+                    xml_deleted_obj_t *deleted_obj = calloc(1, sizeof(xml_deleted_obj_t));
+
                     crm_trace("Deleting %s %p from %p", buffer, child, doc);
+
+                    deleted_obj->path = strdup(buffer);
+
+                    deleted_obj->position = -1;
+                    /* Record the "position" only for XML comments for now */
+                    if (child->type == XML_COMMENT_NODE) {
+                        if (position >= 0) {
+                            deleted_obj->position = position;
+
+                        } else {
+                            deleted_obj->position = __xml_offset(child);
+                        }
+                    }
+
                     p = doc->_private;
-                    p->deleted_paths = g_list_append(p->deleted_paths, strdup(buffer));
+                    p->deleted_objs = g_list_append(p->deleted_objs, deleted_obj);
                     set_doc_flag(child, xpf_dirty);
                 }
             }
@@ -2629,6 +2700,13 @@ free_xml(xmlNode * child)
     }
 }
 
+
+void
+free_xml(xmlNode * child)
+{
+    free_xml_with_position(child, -1);
+}
+
 xmlNode *
 copy_xml(xmlNode * src)
 {
@@ -4039,7 +4117,8 @@ __xml_diff_object(xmlNode * old, xmlNode * new)
 
             __xml_node_clean(candidate);
             __xml_acl_apply(top); /* Make sure any ACLs are applied to 'candidate' */
-            free_xml(candidate);
+            /* Record the old position */
+            free_xml_with_position(candidate, __xml_offset(old_child));
 
             if (find_element(new, old_child, TRUE) == NULL) {
                 xml_private_t *p = old_child->_private;

Places

File bug-1024037_pacemaker-libcrmcommon-delete-XML-comments.patch of Package pacemaker.8397

Places