Eric Blake <ebl...@redhat.com> writes:
> On 02/14/2017 04:26 AM, Markus Armbruster wrote:
>> This will permit its use in parse_option_size().
>
> Progress! (not that it matters - off_t is a signed value, so you are
> STILL limited to 2^63, not 2^64, as the maximum theoretical size storage
> that you can target - and it's not like we have that much storage even
> accessible)
>
>>
>> Cc: Dr. David Alan Gilbert <dgilb...@redhat.com>
>> Cc: Eduardo Habkost <ehabk...@redhat.com> (maintainer:X86)
>> Cc: Kevin Wolf <kw...@redhat.com> (supporter:Block layer core)
>> Cc: Max Reitz <mre...@redhat.com> (supporter:Block layer core)
>> Cc: qemu-bl...@nongnu.org (open list:Block layer core)
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>
>> +++ b/hmp.c
>> @@ -1338,7 +1338,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const
>> QDict *qdict)
>> {
>> const char *param = qdict_get_str(qdict, "parameter");
>> const char *valuestr = qdict_get_str(qdict, "value");
>> - int64_t valuebw = 0;
>> + uint64_t valuebw = 0;
>> long valueint = 0;
>> Error *err = NULL;
>> bool use_int_value = false;
>> @@ -1379,7 +1379,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const
>> QDict *qdict)
>> case MIGRATION_PARAMETER_MAX_BANDWIDTH:
>> p.has_max_bandwidth = true;
>> ret = qemu_strtosz_mebi(valuestr, NULL, &valuebw);
>> - if (ret < 0 || (size_t)valuebw != valuebw) {
>> + if (ret < 0 || valuebw > INT64_MAX
>> + || (size_t)valuebw != valuebw) {
>
> I don't know if this looks any better as:
>
> if (ret < 0 || valuebw > MIN(INT64_MAX, SIZE_MAX))
>
> so don't bother changing it if you think that's ugly.
I think (size_t)valuebw != valuebw neatly expresses "valuebw not
representable as size_t". Requires unsigned, though.
>> +++ b/qapi/opts-visitor.c
>> @@ -481,7 +481,6 @@ opts_type_size(Visitor *v, const char *name, uint64_t
>> *obj, Error **errp)
>> {
>> OptsVisitor *ov = to_ov(v);
>> const QemuOpt *opt;
>> - int64_t val;
>> int err;
>>
>> opt = lookup_scalar(ov, name, errp);
>> @@ -489,14 +488,13 @@ opts_type_size(Visitor *v, const char *name, uint64_t
>> *obj, Error **errp)
>> return;
>> }
>>
>> - err = qemu_strtosz(opt->str ? opt->str : "", NULL, &val);
>> + err = qemu_strtosz(opt->str ? opt->str : "", NULL, obj);
>> if (err < 0) {
>> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
>> - "a size value representible as a non-negative int64");
>
> Nice - you're killing the unusual variant spelling of representable.
>
>> +++ b/util/cutils.c
>> @@ -207,7 +207,7 @@ static int64_t suffix_mul(char suffix, int64_t unit)
>> */
>> static int do_strtosz(const char *nptr, char **end,
>> const char default_suffix, int64_t unit,
>> - int64_t *result)
>> + uint64_t *result)
>> {
>> int retval;
>> char *endptr;
>> @@ -237,7 +237,11 @@ static int do_strtosz(const char *nptr, char **end,
>> retval = -EINVAL;
>> goto out;
>> }
>> - if ((val * mul >= INT64_MAX) || val < 0) {
>> + /*
>> + * Values >= 0xfffffffffffffc00 overflow uint64_t after their trip
>> + * through double (53 bits of precision).
>> + */
>> + if ((val * mul >= 0xfffffffffffffc00) || val < 0) {
>
> I guess there's not any simpler expression using INT64_MAX and
> operations (including casts to double and int64_t) that would reliably
> produce the same constant that you just open-coded here.
I think the literal plus the comment explaining it is easier to read.
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
