Eric Blake <ebl...@redhat.com> writes: > On 11/20/18 3:25 AM, David Hildenbrand wrote: >> Let's provide a wrapper for strtod(). >> >> Reviewed-by: Eric Blake <ebl...@redhat.com> > > This changed enough from v1 that I would have dropped R-b to ensure > that reviewers notice the differences. > >> Signed-off-by: David Hildenbrand <da...@redhat.com> >> --- >> include/qemu/cutils.h | 2 ++ >> util/cutils.c | 65 +++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 67 insertions(+) >> > >> + * If the conversion overflows, store +/-HUGE_VAL in @result, depending >> + * on the sign, and return -ERANGE. >> + * >> + * If the conversion underflows, store ±0.0 in @result, depending on the >> + * sign, and return -ERANGE. > > The use of UTF-8 ± in one place but not both is odd. I think we're at > the point where UTF-8 comments are acceptable these days, rather than > trying to keep our codebase ASCII-clean, so I don't care which way you > resolve the inconsistency.
217 out of 6455 git-controlled files contain non-ASCII characters. 53 of them are binary, and don't count. In most text files, it's for spelling names of authors properly in comments. Ample precedence for UTF-8 in comments, I'd say. That said, I second Eric's call for consistency, with the slightest of preferrences for plain ASCII. I spotted UTF-8 in two error messages, which might still be unadvisable: hw/misc/tmp105.c: error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range", hw/misc/tmp421.c: error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C is out of range", >> +/** >> + * Convert string @nptr to a finite double. >> + * >> + * Works like qemu_strtod(), except that "NaN" and "inf" are rejected >> + * with -EINVAL and no conversion is performed. >> + */ >> +int qemu_strtod_finite(const char *nptr, const char **endptr, double >> *result) >> +{ >> + double tmp; >> + int ret; >> + >> + ret = qemu_strtod(nptr, endptr, &tmp); >> + if (ret) { >> + return ret; > > So, if we overflow, we are returning -ERANGE but with nothing stored > into *result. This is different from qemu_strtod(), where a return of > -ERANGE guarantees that *result is one of 4 values (+/- 0.0/inf). > That seems awkward. Violates the contract's "like qemu_strtod()". >> + } else if (!isfinite(tmp)) { >> + if (endptr) { >> + *endptr = nptr; >> + } >> + return -EINVAL; > > Rewinding back to the start of "inf" is interesting, but matches your > documentation. Yes. I like it. >> + } >> + >> + *result = tmp; >> + return ret; >> +} >> + > > I think you still need to fix -ERANGE handling before I can give R-b.