On Fri, Nov 08, 2019 at 09:05:52AM +0100, Markus Armbruster wrote:
Tao Xu <address@hidden> writes:
On 11/7/2019 9:31 PM, Eduardo Habkost wrote:
On Thu, Nov 07, 2019 at 02:24:52PM +0800, Tao Xu wrote:
On 11/7/2019 4:53 AM, Eduardo Habkost wrote:
On Mon, Oct 28, 2019 at 03:52:12PM +0800, Tao Xu wrote:
Add tests for time input such as zero, around limit of precision,
signed upper limit, actual upper limit, beyond limits, time suffixes,
and etc.
Signed-off-by: Tao Xu <address@hidden>
---
[...]
+ /* Close to signed upper limit 0x7ffffffffffffc00 (53 msbs set) */
+ qdict = keyval_parse("time1=9223372036854774784," /* 7ffffffffffffc00 */
+ "time2=9223372036854775295", /* 7ffffffffffffdff */
+ NULL, &error_abort);
+ v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+ qobject_unref(qdict);
+ visit_start_struct(v, NULL, NULL, 0, &error_abort);
+ visit_type_time(v, "time1", &time, &error_abort);
+ g_assert_cmphex(time, ==, 0x7ffffffffffffc00);
+ visit_type_time(v, "time2", &time, &error_abort);
+ g_assert_cmphex(time, ==, 0x7ffffffffffffc00);
I'm confused by this test case and the one below[1]. Are these
known bugs? Shouldn't we document them as known bugs?
Because do_strtosz() or do_strtomul() actually parse with strtod(), so the
precision is 53 bits, so in these cases, 7ffffffffffffdff and
fffffffffffffbff are rounded.
My questions remain: why isn't this being treated like a bug?
Hi Markus,
I am confused about the code here too. Because in do_strtosz(), the
upper limit is
val * mul >= 0xfffffffffffffc00
So some data near 53 bit may be rounded. Is there a bug?
No, but the design is surprising, and the functions lack written
contracts, except for the do_strtosz() helper, which has one that sucks.
qemu_strtosz() & friends are designed to accept fraction * unit
multiplier. Example: 1.5M means 1.5 * 1024 * 1024 with qemu_strtosz()
and qemu_strtosz_MiB(), and 1.5 * 1000 * 1000 with
qemu_strtosz_metric(). Whether supporting fractions is a good idea is
debatable, but it's what we've got.
The implementation limits the numeric part to the precision of double,
i.e. 53 bits. "8PiB should be enough for anybody."
Switching it from double to long double raises the limit to the
precision of long double. At least 64 bit on common hosts, but hosts
exist where it's the same 53 bits. Do we support any such hosts? If
yes, then we'd make the precision depend on the host, which feels like a
bad idea.
A possible alternative is to parse the numeric part both as a double and
as a 64 bit unsigned integer, then use whatever consumes more
characters. This enables providing full 64 bits unless you actually use
a fraction.
This sounds like the right thing to do, if user input is an
integer and the code in the other end is consuming an integer.
As far as I remember, the only problem we've ever had with the 53 bits
limit is developer confusion :)
Developer confusion, I can deal with. However, exposing this
behavior on external interfaces is a bug to me.
I don't know how serious the bug is because I don't know which
interfaces are affected by it. Do we have a list?
Patches welcome.
My first goal is to get the maintainers of that code to recognize
it as a bug. Then I hope this will motivate somebody else to fix
it. :)