From: Upasana Date: Wed, 9 Jul 2014 21:25:11 +0000 (+0530) Subject: rrd_strtoding now takes error string as an argument X-Git-Tag: v1.5.0-rc1~63^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=89b23aa6394f0c4f0a769a34d8791af0d7a58a21;p=thirdparty%2Frrdtool-1.x.git 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 035b0eea..a5f22579 100644 --- 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 50d7d2ae..185e2d4d 100644 --- 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 3423f0e7..aa4df9f1 100644 --- 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 3c2a2a61..2d73fc0f 100644 --- 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 58ac6adb..3043cf5e 100644 --- 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, ¶m ); + rrd_strtoding( double_str, 0, ¶m, "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 c42de4a5..ac22de0a 100644 --- 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 4756febe..bfdfa73c 100644 --- 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 a35a8804..12935327 100644 --- 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 4ea8205e..9c5fe0ea 100644 --- 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 145c9bda..49b50dd8 100644 --- 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 f7f3c11c..f95361df 100644 --- 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, ¶m, "Error while parsing Holt-Winters parameter"); /* read the value */ - rrd_strtoding(arg, 0, ¶m); - 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, ¶m); + strtod_ret_val = rrd_strtoding(arg, 0, ¶m, "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, ¶m); - if (param < 0.1) { + strtod_ret_val = rrd_strtoding(arg, 0, ¶m, "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 d75c61db..eb994bc7 100644 --- 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;