rrd_strtoding now takes error string as an argument

This commit replaces all rrd_strtoding calls with a 4 argument
version of the function and also make sure that its return value
(which shows whether the string has been converted partially,
fully or not converted at all) is checked at all the places.

diff --git a/src/rrd_client.c b/src/rrd_client.c
index 035b0eea031b229386698637eb72ffd1a3039c6a..a5f225792e363ff2df2b1b5a07bf04d0cec46edb 100644 (file)
--- a/src/rrd_client.c
+++ b/src/rrd_client.c
@@ -269,7 +269,7 @@ static int parse_value_array_header (char *line, /* {{{ */
    * will expect a comma as the decimal separator, i.e. "42,77". */
   for (i = 0; i < array_len; i++)
   {
-    if( rrd_strtoding(str_array[i], 0, &tmp) == 2) {
+    if( rrd_strtoding(str_array[i], 0, &tmp, "parse_value_array_header") == 2) {
         array[i] = (rrd_value_t)tmp;
     } else {
         free(str_array);
@@ -1586,7 +1586,8 @@ int rrdc_stats_get (rrdc_stats_t **ret_stats) /* {{{ */
         || (strcmp ("TreeNodesNumber", key) == 0))
     {
       s->type = RRDC_STATS_TYPE_GAUGE;
-      rrd_strtoding(value, &endptr, &(s->value.gauge));
+      rrd_strtoding(value, &endptr, &(s->value.gauge),
+                                    "QueueLength or TreeDepth or TreeNodesNumber");
     }
     else if ((strcmp ("DataSetsWritten", key) == 0)
         || (strcmp ("FlushesReceived", key) == 0)

diff --git a/src/rrd_create.c b/src/rrd_create.c
index 50d7d2ae94be9872f226510bab725db3922df54a..185e2d4d0e5d01a58589c2f2eab1b56edd87b3ff 100644 (file)
--- a/src/rrd_create.c
+++ b/src/rrd_create.c
@@ -666,17 +666,14 @@ void parseGENERIC_DS(
             if (minstr[0] == 'U' && minstr[1] == 0)
                 ds_def->par[DS_min_val].u_val = DNAN;
             else
-                rrd_strtoding(minstr, 0, &(ds_def->par[DS_min_val].u_val) );
+                if( rrd_strtoding(minstr, 0, &(ds_def->par[DS_min_val].u_val), "parsing min val") != 2 ) return;
 
             if (maxstr[0] == 'U' && maxstr[1] == 0)
                 ds_def->par[DS_max_val].u_val = DNAN;
             else
-                rrd_strtoding(maxstr, 0, &(ds_def->par[DS_max_val].u_val) );
+                if( rrd_strtoding(maxstr, 0, &(ds_def->par[DS_max_val].u_val), "parsing max val") != 2 ) return;
 
-            if (!isnan(ds_def->par[DS_min_val].u_val) &&
-                !isnan(ds_def->par[DS_max_val].u_val) &&
-                ds_def->par[DS_min_val].u_val
-                >= ds_def->par[DS_max_val].u_val) {
+            if ( ds_def->par[DS_min_val].u_val >= ds_def->par[DS_max_val].u_val ) {
                 parsetime_error = "min must be less than max in DS definition";
                 break;
             }

diff --git a/src/rrd_daemon.c b/src/rrd_daemon.c
index 3423f0e7e64d514b9aa1b6a8236aec505f36987d..aa4df9f1835a9f2898bc27bb94310d835eca79e5 100644 (file)
--- a/src/rrd_daemon.c
+++ b/src/rrd_daemon.c
@@ -1512,7 +1512,7 @@ static int handle_request_update (HANDLER_PROTO) /* {{{ */
 
     /* make sure update time is always moving forward. We use double here since
        update does support subsecond precision for timestamps ... */
-    if ( ( rrd_strtoding( value, &eostamp, &stamp ) != 2 ) || *eostamp != ':')
+    if ( ( rrd_strtoding( value, &eostamp, &stamp, "error while parsing time stamp" ) != 2 ) || *eostamp != ':')
     {
       pthread_mutex_unlock(&cache_lock);
       return send_response(sock, RESP_ERR,

diff --git a/src/rrd_fetch_libdbi.c b/src/rrd_fetch_libdbi.c
index 3c2a2a6142fb321caaca543e7e7214d72d1541f8..2d73fc0fefa55a626c6b786fabc4447cb8f7d3b0 100644 (file)
--- a/src/rrd_fetch_libdbi.c
+++ b/src/rrd_fetch_libdbi.c
@@ -95,14 +95,20 @@ static double rrd_fetch_dbi_double(dbi_result *result,int idx) {
   /* get the attributes for this filed */
   unsigned int attr=dbi_result_get_field_attribs_idx(result,idx);
   unsigned int type=dbi_result_get_field_type_idx(result,idx);
+  unsigned int strtod_ret_val;
   /* return NAN if NULL */
   if(dbi_result_field_is_null_idx(result,idx)) { return DNAN; }
   /* do some conversions */
   switch (type) {
     case DBI_TYPE_STRING:
       ptmp=(char*)dbi_result_get_string_idx(result,idx);
-      rrd_strtoding(ptmp,NULL, &value);
-      break;
+      strtod_ret_val = rrd_strtoding(ptmp,NULL, &value, "rrd_fetch_dbi_double, DBI_TYPE_STRING");
+      if( strtod_ret_val == 1 || strtod_ret_val == 2 ) {
+        break;
+      } else {
+        value = DNAN;
+        break;
+      }
     case DBI_TYPE_INTEGER:
       if        (attr & DBI_INTEGER_SIZE1) { value=dbi_result_get_char_idx(result,idx);
       } else if (attr & DBI_INTEGER_SIZE2) { value=dbi_result_get_short_idx(result,idx);
@@ -131,7 +137,7 @@ static double rrd_fetch_dbi_double(dbi_result *result,int idx) {
        }
       }
       /* convert to number */
-      rrd_strtoding(ptmp,NULL, &value);
+      strtod_ret_val = rrd_strtoding(ptmp,NULL, &value, "rrd_fetch_dbi_double, DBI_TYPE_BINARY");
       /* free pointer */
       free(ptmp);
       break;

diff --git a/src/rrd_graph.c b/src/rrd_graph.c
index 58ac6adbcac6694e3e30b9045b46eb79c215ec1e..3043cf5eddf17be467a2b23d68c061d46192c7fa 100644 (file)
--- a/src/rrd_graph.c
+++ b/src/rrd_graph.c
@@ -4583,7 +4583,7 @@ void rrd_graph_options(
             im->forceleftspace = 1;
             break;
         case 'T':
-            rrd_strtoding(optarg, 0, &(im->tabwidth) );
+            rrd_strtoding(optarg, 0, &(im->tabwidth), "Function rrd_graph_options, option 'T'");
             break;
         case 'S':
             im->step = atoi(optarg);
@@ -4652,7 +4652,7 @@ void rrd_graph_options(
                 break;
             };
             if (sscanf(optarg, "%[-0-9.e+]:%d", double_str , &im->ylabfact) == 2) {
-                rrd_strtoding( double_str, 0, &(im->ygridstep) );
+                rrd_strtoding( double_str, 0, &(im->ygridstep), "Function rrd_graph_options, option 'y'");
                 if (im->ygridstep <= 0) {
                     rrd_set_error("grid step must be > 0");
                     return;
@@ -4673,8 +4673,8 @@ void rrd_graph_options(
                       "%[-0-9.e+]:%[-0-9.e+]",
                       double_str,
                       double_str2 ) != 2) {
-                rrd_strtoding( double_str, 0, &(im->grid_dash_on) );
-                rrd_strtoding( double_str2, 0, &(im->grid_dash_off)  );
+                rrd_strtoding( double_str, 0, &(im->grid_dash_on), "Function rrd_graph_options, option 1008");
+                rrd_strtoding( double_str2, 0, &(im->grid_dash_off), "Function rrd_graph_options, option 1008");
                 rrd_set_error("expected grid-dash format float:float");
                 return;
             }
@@ -4691,8 +4691,8 @@ void rrd_graph_options(
                       "%[-0-9.e+]:%[-0-9.e+]",
                       double_str,
                       double_str2 ) == 2) {
-                rrd_strtoding( double_str, 0, &(im->second_axis_scale) );
-                rrd_strtoding( double_str2, 0, &(im->second_axis_shift) );
+                rrd_strtoding( double_str, 0, &(im->second_axis_scale), "Function rrd_graph_options, option 1002");
+                rrd_strtoding( double_str2, 0, &(im->second_axis_shift), "Function rrd_graph_options, option 1002");
                 if(im->second_axis_scale==0){
                     rrd_set_error("the second_axis_scale  must not be 0");
                     return;
@@ -4739,10 +4739,10 @@ void rrd_graph_options(
             }
             break;
         case 'u':
-            rrd_strtoding(optarg, 0, &(im->maxval));
+            rrd_strtoding(optarg, 0, &(im->maxval), "Function rrd_graph_options, option 'u'");
             break;
         case 'l':
-            rrd_strtoding(optarg, 0, &(im->minval));
+            rrd_strtoding(optarg, 0, &(im->minval), "Function rrd_graph_options, option 'l'");
             break;
         case 'b':
             im->base = atol(optarg);
@@ -4855,7 +4855,7 @@ void rrd_graph_options(
             int       end;
 
             if (sscanf(optarg, "%10[A-Z]:%[-0-9.e+]%n", prop, double_str, &end) >= 2) {
-                rrd_strtoding( double_str, 0, &size );
+                rrd_strtoding( double_str, 0, &size, "Function rrd_graph_options, option 'n'" );
                 int       sindex, propidx;
 
                 if ((sindex = text_prop_conv(prop)) != -1) {
@@ -4890,7 +4890,7 @@ void rrd_graph_options(
             break;
         }
         case 'm':
-            rrd_strtoding(optarg, 0, &(im->zoom));
+            rrd_strtoding(optarg, 0, &(im->zoom), "Function rrd_graph_options, option 'm'");
             if (im->zoom <= 0.0) {
                 rrd_set_error("zoom factor must be > 0");
                 return;
@@ -5162,7 +5162,7 @@ int vdef_parse(
 
     n = 0;
     sscanf(str, "%[-0-9.e+],%29[A-Z]%n", double_str, func, &n);
-    rrd_strtoding( double_str, 0, &param );
+    rrd_strtoding( double_str, 0, &param, "Function vdef_parse" );
     if (n == (int) strlen(str)) {   /* matched */
         ;
     } else {

diff --git a/src/rrd_graph_helper.c b/src/rrd_graph_helper.c
index c42de4a5127016d040e0aaa7e869097aea28fa93..ac22de0a82a4c6fd5fffcacc0926ebe2d46cb0a7 100644 (file)
--- a/src/rrd_graph_helper.c
+++ b/src/rrd_graph_helper.c
@@ -144,11 +144,11 @@ int getDouble(const char* v, double *val,char**extra) {
   unsigned int strtod_ret;
   *extra=NULL;
 
-  if( rrd_strtoding( v, extra, val ) != 2 ) {
+  if( rrd_strtoding( v, extra, val, "Function getDouble" ) != 2 ) {
     return -1;
   }
 
-  strtod_ret = rrd_strtoding( v, extra, val );
+  strtod_ret = rrd_strtoding( v, extra, val, "Function getDouble" );
 
   /* see rrd_strtoding's return values for more infromation */
   if( strtod_ret == 0 )

diff --git a/src/rrd_restore.c b/src/rrd_restore.c
index 4756febe34d60d116eb8bba4b9e113b9f48595d1..bfdfa73c686d32deb888ad873f29e2ec77548b12 100644 (file)
--- a/src/rrd_restore.c
+++ b/src/rrd_restore.c
@@ -329,7 +329,7 @@ static int get_xml_double(
             xmlFree(text);
             return 0;            
         }        
-        if ( rrd_strtoding((char *)text,NULL, &temp) != 2 ){
+        if ( rrd_strtoding((char *)text,NULL, &temp, "Function xml_get_double") != 2 ){
             rrd_set_error("ling %d: get_xml_double from '%s' %s",
                           xmlTextReaderGetParserLineNumber(reader),
                           text,rrd_strerror(errno));

diff --git a/src/rrd_rpncalc.c b/src/rrd_rpncalc.c
index a35a88049318342eecf5ac50e02731f9b850834e..129353278e92891335732a066807603ec025854f 100644 (file)
--- a/src/rrd_rpncalc.c
+++ b/src/rrd_rpncalc.c
@@ -317,8 +317,8 @@ rpnp_t   *rpn_parse(
         }
 
         else if ((sscanf(expr, "%[-0-9.e+]%n", double_str, &pos) == 1)
-                 && (expr[pos] == ',')) {
-            rrd_strtoding( double_str, 0, &(rpnp[steps].val) );
+                 && (expr[pos] == ',')
+                 && ( rrd_strtoding( double_str, 0, &(rpnp[steps].val), "Error while parsing double in rpn_parse" ) == 2 )) {
             rpnp[steps].op = OP_NUMBER;
             expr += pos;
         }

diff --git a/src/rrd_strtod.c b/src/rrd_strtod.c
index 4ea8205e793b6bd5596d7d83e5dcc264024cda82..9c5fe0eaa0d21df1cd03ca1737e4e6f87fc1a948 100644 (file)
--- a/src/rrd_strtod.c
+++ b/src/rrd_strtod.c
@@ -44,26 +44,44 @@
 
 /* returns 2 on success */
 /* i.e. if the whole string has been converted to a double successfully */
-unsigned int rrd_strtoding(const char * str, char ** endptr, double * dbl) {
-    *dbl = rrd_strtod( str, endptr );
+unsigned int rrd_strtoding
+(const char * str, char ** endptr, double * dbl, char * error) {
+    char * local_endptr;
+    *dbl = rrd_strtod( str, &local_endptr );
 
-    if( endptr != NULL && *endptr == str ) {
+    if( endptr != NULL ) endptr = &local_endptr;
+
+    if( local_endptr == str ) {
         /* no conversion has been done */
         /* for inputs like "abcdj", i.e. no number at all */
-        rrd_set_error("Cannot convert %s to float", str);
+        if( error == NULL ) {
+            rrd_set_error("Cannot convert %s to float", str);
+        } else {
+            rrd_set_error("%s - Cannot convert %s to float", error, str);
+        }
         return 0;
-    } else if( endptr != NULL && endptr[0] != '\0' ) {
+    } else if( local_endptr[0] != '\0' ) {
         /* conversion has been done, but whole string is not a number */
         /* for inputs like "33.343djdjk" */
-        rrd_set_error("Cannot convert %s to float", str);
+        if( error == NULL ) {
+            rrd_set_error("Converted %s to %lf, but cannot convert %s",
+                           str, *dbl, local_endptr);
+        } else {
+            rrd_set_error("%s - Converted %s (%s) to %lf, but cannot convert %s",
+                           error, str, *dbl, local_endptr);
+        }
         return 1;
-    } else if( endptr != NULL && endptr[0] == '\0' ) {
+    } else if( local_endptr[0] == '\0' ) {
         /* conversion successfully done */
         /* for inputs that are totally numbers "23.343" */
         return 2;
     } else {
-        /* just to be safe */
-        rrd_set_error("..");
+      /* just to be safe */
+        if( error == NULL )
+            rrd_set_error("Internal error. Something is seriously wrong '%s'", str);
+        else
+            rrd_set_error("%s - Internal error. Something is seriously wrong '%s'",error, str);
+
         return 3;
     }
 }

diff --git a/src/rrd_strtod.h b/src/rrd_strtod.h
index 145c9bdaf1862e38d99f4be13863644fe12eec07..49b50dd8bba1d451f8843491868d0ad22c822ff4 100644 (file)
--- a/src/rrd_strtod.h
+++ b/src/rrd_strtod.h
@@ -1,2 +1,2 @@
-unsigned int rrd_strtoding(const char * str, char ** endptr, double * dbl);
+unsigned int rrd_strtoding(const char * str, char ** endptr, double * dbl, char * error);
 double rrd_strtod(const char *str, char **endptr);

diff --git a/src/rrd_tune.c b/src/rrd_tune.c
index f7f3c11cb649321acde3029990edc36dbd2f0994..f95361dfda9fcbe460f0d3f102e37775ed8e5c0c 100644 (file)
--- a/src/rrd_tune.c
+++ b/src/rrd_tune.c
@@ -184,6 +184,7 @@ int rrd_tune(
     while (1) {
         int       option_index = 0;
         int       opt;
+        unsigned int strtod_ret_val;
 
         opt = getopt_long(argc, argv, "h:i:a:d:r:p:n:w:f:x:y:z:v:b:",
                           long_options, &option_index);
@@ -206,12 +207,16 @@ int rrd_tune(
             break;
 
         case 'i':
-            if ((matches =
-                 sscanf(optarg, DS_NAM_FMT ":%[-0-9.e+]", ds_nam, double_str)) < 1) {
+            matches = sscanf(optarg, DS_NAM_FMT ":%[-0-9.e+]", ds_nam, double_str);
+            if( matches >= 1 ) {
+                strtod_ret_val = rrd_strtoding( double_str, 0, &min, "Function rrd_tune, option i" );
+            }
+
+            if ((matches < 1) || (strtod_ret_val != 2)) {
                 rrd_set_error("invalid arguments for minimum ds value");
                goto done;
             }
-            rrd_strtoding( double_str, 0, &min );
+
             if ((ds = ds_match(&rrd, ds_nam)) == -1) {
                goto done;
             }
@@ -222,15 +227,20 @@ int rrd_tune(
             break;
 
         case 'a':
-            if ((matches =
-                 sscanf(optarg, DS_NAM_FMT ":%[-0-9.e+]", ds_nam, double_str)) < 1) {
+            matches = sscanf(optarg, DS_NAM_FMT ":%[-0-9.e+]", ds_nam, double_str);
+            if( matches >= 1 ) {
+                strtod_ret_val = rrd_strtoding( double_str, 0, &max, "Function rrd_tune, option i" );
+            }
+
+            if ((matches < 1 ) || (strtod_ret_val != 2)) {
                 rrd_set_error("invalid arguments for maximum ds value");
                goto done;
             }
-            rrd_strtoding( double_str, 0, &max );
+
             if ((ds = ds_match(&rrd, ds_nam)) == -1) {
                goto done;
             }
+
             if (matches == 1)
                 max = DNAN;
             rrd.ds_def[ds].par[DS_max_val].u_val = max;
@@ -418,12 +428,17 @@ int set_hwarg(
     double    param;
     unsigned long i;
     signed short rra_idx = -1;
+    unsigned int strtod_ret_val;
 
+    strtod_ret_val = rrd_strtoding(arg, 0, &param, "Error while parsing Holt-Winters parameter");
     /* read the value */
-    rrd_strtoding(arg, 0, &param);
-    if (param <= 0.0 || param >= 1.0) {
+    if ((strtod_ret_val == 1 || strtod_ret_val == 2 ) &&
+         (param <= 0.0 || param >= 1.0) ) {
         rrd_set_error("Holt-Winters parameter must be between 0 and 1");
         return -1;
+    } else if( strtod_ret_val == 0 || strtod_ret_val > 2 ) {
+        rrd_set_error("Unable to parse Holt-Winters parameter");
+        return -1;
     }
     /* does the appropriate RRA exist?  */
     for (i = 0; i < rrd->stat_head->rra_cnt; ++i) {
@@ -451,14 +466,19 @@ int set_hwsmootharg(
     double    param;
     unsigned long i;
     signed short rra_idx = -1;
+    unsigned int strtod_ret_val;
 
     /* read the value */
-    rrd_strtoding(arg, 0, &param);
+    strtod_ret_val = rrd_strtoding(arg, 0, &param, "Error while parsing Holt-Winters parameter, function set_hesmootharg");
     /* in order to avoid smoothing of SEASONAL or DEVSEASONAL, we need to 
      * the 0.0 value*/
-    if (param < 0.0 || param > 1.0) {
+    if ( (strtod_ret_val == 1 || strtod_ret_val == 2 ) &&
+         (param < 0.0 || param > 1.0) ) {
         rrd_set_error("Holt-Winters parameter must be between 0 and 1");
         return -1;
+    } else if( strtod_ret_val == 0 || strtod_ret_val > 2 ) {
+        rrd_set_error("Unable to parse Holt-Winters parameter");
+        return -1;
     }
     /* does the appropriate RRA exist?  */
     for (i = 0; i < rrd->stat_head->rra_cnt; ++i) {
@@ -485,12 +505,18 @@ int set_deltaarg(
     rrd_value_t param;
     unsigned long i;
     signed short rra_idx = -1;
+    unsigned int strtod_ret_val;
 
-    rrd_strtoding(arg, 0, &param);
-    if (param < 0.1) {
+    strtod_ret_val = rrd_strtoding(arg, 0, &param, "Function set_deltaarg" );
+    if ((strtod_ret_val == 1 || strtod_ret_val == 2) &&
+         param < 0.1) {
         rrd_set_error("Parameter specified is too small");
         return -1;
+    } else if( strtod_ret_val == 1 || strtod_ret_val > 2 ) {
+        rrd_set_error("Unable to parse parameter in set_deltaarg");
+        return -1;
     }
+
     /* does the appropriate RRA exist?  */
     for (i = 0; i < rrd->stat_head->rra_cnt; ++i) {
         if (cf_conv(rrd->rra_def[i].cf_nam) == CF_FAILURES) {

diff --git a/src/rrd_update.c b/src/rrd_update.c
index d75c61db246f504e73317489b34b7f48607e25a5..eb994bc71d6a928c1dfd1f7d4554f072446956f9 100644 (file)
--- a/src/rrd_update.c
+++ b/src/rrd_update.c
@@ -1317,7 +1317,7 @@ static int get_time_from_reading(
         *current_time = tmp_time.tv_sec;
         *current_time_usec = tmp_time.tv_usec;
     } else {
-        if ( rrd_strtoding( updvals[0], 0, &tmp) != 2) {
+        if ( rrd_strtoding( updvals[0], 0, &tmp, "error while parsing time in get_time_from_reading") != 2) {
             return -1;
         };
         if (tmp < 0.0){
@@ -1422,13 +1422,13 @@ static int update_pdp_prep(
                 }
                 break;
             case DST_ABSOLUTE:
-                if( rrd_strtoding(updvals[ds_idx + 1], 0, &pdp_new[ds_idx] ) != 2 ) {
+                if( rrd_strtoding(updvals[ds_idx + 1], 0, &pdp_new[ds_idx], "Function update_pdp_prep, case DST_ABSOLUTE" ) != 2 ) {
                     return -1;
                 }
                 rate = pdp_new[ds_idx] / interval;
                 break;
             case DST_GAUGE:
-                if( rrd_strtoding( updvals[ds_idx + 1], 0, &tmp ) == 2 ) {
+                if( rrd_strtoding( updvals[ds_idx + 1], 0, &tmp, "Function update_pdp_prep, case DST_GAUGE") == 2 ) {
                     pdp_new[ds_idx] = tmp * interval;
                 } else {
                     return -1;