On 06/24/2017 08:15 PM, Julio Faracco wrote: > There are no occurrences of tests related to Strings and Double numbers > inside virstringtest.c. This commit introduces some tests to validate the > conversion. The test does not include locale changes yet. > > Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com&gt; > --- > tests/virstringtest.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/tests/virstringtest.c b/tests/virstringtest.c > index 97c6e76..32ce79c 100644 > --- a/tests/virstringtest.c > +++ b/tests/virstringtest.c > @@ -652,6 +652,52 @@ testStringToLong(const void *opaque) > } > > > +struct stringToDoubleData { > + const char *str; > + const char *end_ptr; > + double res; > +}; > + > +/* This test checks if double strings are successfully converted to double > + * number considering the byproduct string too. */ > +static int > +testStringToDouble(const void *opaque) > +{ > + const struct stringToDoubleData *data = opaque; > + int ret = -1; > + char *end_ptr = NULL; > + double res = 0; > + > + if (data->end_ptr) { > + ret = virStrToDouble(data->str, &end_ptr, &res); > + } else { > + /* end_ptr returns or a substring or an empty string. > + * It never returns a NULL pointer. */ > + ret = virStrToDouble(data->str, NULL, &res); > + } Not sure the comment makes sense... Why not just one line: ret = virStrToDouble(data->str, data->end_ptr ? &end_ptr : NULL, &res); or combining with the subsequent "if (ret < 0) {" test: if ((ret = virStrToDouble(data->str, data->end_ptr ? &end_ptr : NULL, &res)) < 0) { > + > + if (ret < 0) { > + fprintf(stderr, "Convert error of '%s', expected '%f'\n", Should the format be %lf or %g? I've see both used within libvirt code - search around for VIR_TYPED_PARAM_DOUBLE, _TYPE_DOUBLE, or "param.d" printing. (similarly for the next %f usage as well) > + data->str, data->res); > + return ret; > + } > + > + if (res != data->res) { > + fprintf(stderr, "Returned '%f', expected '%f'\n", > + res, data->res); > + return -1; > + } > + > + /* Comparing substrings. */ > + if (STRNEQ_NULLABLE(end_ptr, data->end_ptr)) { > + fprintf(stderr, "Expected substring '%s', but got '%s'\n", > + end_ptr, data->end_ptr); > + return -1; > + } > + > + return ret; > +} > + > /* The point of this test is to check whether all members of the array are > * freed. The test has to be checked using valgrind. */ > static int > @@ -965,6 +1011,44 @@ mymain(void) > TEST_STRTOL("-18446744073709551616", NULL, 0, -1, 0U, -1, > 0LL, -1, 0ULL, -1); > > +#define TEST_STRTOD(str, end_ptr, res) \ > + do { \ > + struct stringToDoubleData data = { \ > + str, end_ptr, res, \ > + }; \ > + if (virTestRun("virStringToDouble '" str "'", \ > + testStringToDouble, &data) < 0) \ > + ret = -1; \ > + } while(0) This fails syntax-check due to no space between while and (0) I can either make the suggested changes (perhaps someone else has a strong feeling of using %g or %lf) or you can post a new patch. Either way is fine. John > + > + /* Simple numbers. */ > + TEST_STRTOD("0.0", NULL, 0); > + TEST_STRTOD("1.0", NULL, 1); > + TEST_STRTOD("3.14159", NULL, 3.14159); > + TEST_STRTOD("0.57721", NULL, 0.57721); > + > + /* Testing ending string. */ > + TEST_STRTOD("2.718", "", 2.718); > + TEST_STRTOD("2.718 281 828 459", " 281 828 459", 2.718); > + TEST_STRTOD("2.718,281,828,459", ",281,828,459", 2.718); > + > + /* Scientific numbers. */ > + TEST_STRTOD("3.14159e+000", NULL, 3.14159); > + TEST_STRTOD("2.00600e+003", NULL, 2006); > + TEST_STRTOD("1.00000e-010", NULL, 1e-010); > + > + /* Negative numbers. */ > + TEST_STRTOD("-1.6180339887", NULL, -1.6180339887); > + TEST_STRTOD("-0.00031e-010", NULL, -0.00031e-010); > + > + /* Long numbers. */ > + TEST_STRTOD("57089907708238388904078437636832797971793838081897.0", > + NULL, > + 57089907708238388904078437636832797971793838081897.0); > + TEST_STRTOD("3.141592653589793238462643383279502884197169399375105", > + NULL, > + 3.141592653589793238462643383279502884197169399375105); > + > /* test virStringListFreeCount */ > if (virTestRun("virStringListFreeCount", testVirStringListFreeCount, > NULL) < 0) >