rgmanager: spots in flattening-related parts

Just two spots I discovered while running through the code for flattening resource trees in cluster.conf. One cosmetic, one little more subtle.

Jan Pokorn? (2):
resrules: fix free while passing the pointer to caller
resrules: print_resource_rule doc comment conformance

  resrules.c |    9 +++------
  1 files changed, 3 insertions(+), 6 deletions(-)

The version ("OCF API Version" as declared in the code) for resource rules cannot be obtained correctly as the memory is being immediately freed before passing up to the caller. What's worse, the caller could then access uninitialized memory through this pointer (print_resource_rule and especially destroy_resource_rule which could lead to crash easily, IMHO).

The patch fixes this, making no difference between success and failure in getting the version. Both should be handled correctly when either dumping resource rule or destroying it.

Aside: was this version field ever actively used or is this a legacy
part not expected to be triggered?
[ I have no test case at hand, this was random spot, sorry. ]

---
  resrules.c |    7 ++-----
  1 files changed, 2 insertions(+), 5 deletions(-)

index cc59e30..581be9e 100644
@@ -205,11 +205,8 @@ _get_version(xmlDocPtr doc, xmlXPathContextPtr ctx, char *base,
  
  	snprintf(xpath, sizeof(xpath), "%s/@version", base);
  	ret = xpath_get_one(doc, ctx, xpath);
-	if (ret) {
-		rr->rr_version = ret;
-		free(ret);
-	}
-	rr->rr_version = NULL;
+	/* NULL or actual result of the query */
+	rr->rr_version = ret;
  }
  
  

(due to previously misconfigured MUA, sorry for inconvenience)

The version ("OCF API Version" as declared in the code) for resource rules cannot be obtained correctly as the memory is being immediately freed before passing up to the caller. What's worse, the caller could then access uninitialized memory through this pointer (e.g., print_resource_rule).

The patch fixes this, making no difference between success and failure in getting the version. Both should be handled correctly when either dumping resource rule or destroying it.

Aside: was this version field ever actively used of is this a legacy
part not expected to be triggered?
[ I have no test case at hand, this was random spot, sorry. ]

---
 resrules.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

index cc59e30..581be9e 100644
@@ -205,11 +205,8 @@ _get_version(xmlDocPtr doc, xmlXPathContextPtr ctx, char *base,
 
 	snprintf(xpath, sizeof(xpath), "%s/@version", base);
 	ret = xpath_get_one(doc, ctx, xpath);
-	if (ret) {
-		rr->rr_version = ret;
-		free(ret);
-	}
-	rr->rr_version = NULL;
+	/* NULL or actual result of the query */
+	rr->rr_version = ret;
 }
 
 

As Ryan noted, this is not true, as the pointer was always set to NULL. So the patch only eliminates hidden NOOP.

-- Jan

---
  resrules.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

index 581be9e..4fcc4f1 100644
@@ -556,7 +556,7 @@ store_childtype(resource_child_t **childp, char *name, int start,
  
  
  /**
-   Print a resource_t structure to stdout
+   Print a resource_t structure to specified stream
  
     @param rr		Resource rule to print.
   */

(when changing doc. comment, do it properly)

---
  resrules.c |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)

index 581be9e..353ead4 100644
@@ -556,9 +556,10 @@ store_childtype(resource_child_t **childp, char *name, int start,
  
  
  /**
-   Print a resource_t structure to stdout
+   Print a resource_t structure to specified stream
  
     @param rr		Resource rule to print.
+   @param fp		Destination stream.
   */
  void
  print_resource_rule(FILE *fp, resource_rule_t *rr)

(when changing doc. comment, do it properly)
(due to previously misconfigured MUA, sorry for inconvenience)

---
 resrules.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

index 581be9e..353ead4 100644
@@ -556,9 +556,10 @@ store_childtype(resource_child_t **childp, char *name, int start,
 
 
 /**
-   Print a resource_t structure to stdout
+   Print a resource_t structure to specified stream
 
    @param rr		Resource rule to print.
+   @param fp		Destination stream.
  */
 void
 print_resource_rule(FILE *fp, resource_rule_t *rr)