From: Peter Stamfest <peter@stamfest.at>
Date: Sun, 2 Mar 2014 20:47:39 +0000 (+0100)
Subject: Fix some commented out code to change RRD version when certain RRA
X-Git-Tag: v1.5.0-rc1~131^2~15
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=14fdc5b87383ebafef452503bfad42a4b0a1f8d8;p=thirdparty%2Frrdtool-1.x.git

Fix some commented out code to change RRD version when certain RRA
definitions require this.
---

diff --git a/src/rrd_create.c b/src/rrd_create.c
index 614d6a4d..1a2fb8dc 100644
--- a/src/rrd_create.c
+++ b/src/rrd_create.c
@@ -189,6 +189,7 @@ int parseDS(const char *def,
 
 int parseRRA(const char *def,
 	     rra_def_t *rra_def, 
+	     rrd_t *rrd,
 	     unsigned long hash) {
     char     *argvcopy;
     char     *tokptr = "";
@@ -196,6 +197,7 @@ int parseRRA(const char *def,
     int       cf_id = -1;
     int       token_min = 4;
     int       row_cnt;
+    char     *require_version = NULL;
 
     memset(rra_def, 0, sizeof(rra_def_t));
 
@@ -211,7 +213,7 @@ int parseRRA(const char *def,
 	    cf_id = cf_conv(rra_def->cf_nam);
 	    switch (cf_id) {
 	    case CF_MHWPREDICT:
-		// FIXME		strcpy(rrd.stat_head->version, RRD_VERSION);    /* MHWPREDICT causes Version 4 */
+		require_version = RRD_VERSION;    /* MHWPREDICT causes Version 4 */
 	    case CF_HWPREDICT:
 		token_min = 5;
 		/* initialize some parameters */
@@ -385,7 +387,7 @@ int parseRRA(const char *def,
 		if (sscanf(token, "smoothing-window=%lf",
 			   &(rra_def->par[RRA_seasonal_smoothing_window].
 			     u_val))) {
-		    // FIXME		     strcpy(rrd.stat_head->version, RRD_VERSION);    /* smoothing-window causes Version 4 */
+		    require_version = RRD_VERSION;    /* smoothing-window causes Version 4 */
 		    if (rra_def->par[RRA_seasonal_smoothing_window].u_val < 0.0
 			|| rra_def->par[RRA_seasonal_smoothing_window].u_val >
 			1.0) {
@@ -444,6 +446,15 @@ int parseRRA(const char *def,
 	rrd_set_error("Expected at least %i arguments for RRA but got %i",token_min,token_idx);
 	return(-1);
     }
+
+    // parsing went well. ONLY THEN are we allowed to produce
+    // additional side effects.
+    if (require_version != NULL) {
+        if (rrd) {
+            strcpy(rrd->stat_head->version, RRD_VERSION);
+        }
+    }
+
 #ifdef DEBUG
     fprintf(stderr,
 	    "Creating RRA CF: %s, dep idx %lu\n",
@@ -589,7 +600,7 @@ int rrd_create_r2(
                 return (-1);
             }
 
-	    parseRRA(argv[i], rrd.rra_def + rrd.stat_head->rra_cnt,
+	    parseRRA(argv[i], rrd.rra_def + rrd.stat_head->rra_cnt, &rrd,
 		     hashed_name);
 
 	    if (rrd_test_error()) {
diff --git a/src/rrd_create.h b/src/rrd_create.h
index 682931b2..52b7ebdd 100644
--- a/src/rrd_create.h
+++ b/src/rrd_create.h
@@ -17,8 +17,19 @@ int parseDS(const char *def,
 	    void *key_hash,
 	    long (*lookup)(void *, char *));
 
+/* Parse a textual RRA definition into rra_def. The rra_def might be
+   disconnected from any RRD. However, because some definitions cause
+   modifications to other parts of an RRD (like the version) it is
+   possible to pass the RRD that is about to hold the RRA. If the
+   definition really is to stay disconnected, it is permitted to pass
+   NULL to the rrd parameter.
+   The hash is a parameter to introduce some randomness to avoid 
+   potential performance problems when doing bulk updates to many RRDs
+   at once.
+*/
 int parseRRA(const char *def,
 	     rra_def_t *rra_def, 
+	     rrd_t *rrd,
 	     unsigned long hash);
 
 rra_def_t *handle_dependent_rras(rra_def_t *rra_def_array, 
diff --git a/src/rrd_modify.c b/src/rrd_modify.c
index f8d06c17..401f2780 100644
--- a/src/rrd_modify.c
+++ b/src/rrd_modify.c
@@ -858,10 +858,12 @@ static int add_rras(rrd_t *out, rra_mod_op_t *rra_mod_ops, int rra_mod_ops_cnt)
 	    rra_def_t rra_def;
 
 	    // the hash doesn't really matter...
-	    if (parseRRA(rra_mod_ops[r].def, &rra_def, 0x123123823123) != 0) {
+	    parseRRA(rra_mod_ops[r].def, &rra_def, out, 0x123123823123);
+
+	    if (rrd_test_error()) {
 		// failed!!!
 		goto done;
-	    }
+            }
 
 	    out->rra_def = copy_over_realloc(out->rra_def, out->stat_head->rra_cnt,
 					    &rra_def, 0,