Re: [Qemu-devel] [PATCH v1 7/7] memory-device: rewrite address assignment using ranges

[David Hildenbrand <david@redhat.com>]

>>      if (hint) {
>> -        new_addr = *hint;
>> -        if (new_addr < address_space_start) {
>> +        if (!range_valid(*hint, size)) {
>>              error_setg(errp, "can't add memory device [0x%" PRIx64 ":0x%" PRIx64
>> -                       "] before 0x%" PRIx64, new_addr, size,
>> -                       address_space_start);
>> +                       "], range overflow", *hint, size);
>>              return 0;
>> -        } else if ((new_addr + size) > address_space_end) {
>> +        }
>> +        range_init(&new, *hint, size);
>> +        if (range_starts_before_range(&new, &as)) {
>> +            error_setg(errp, "can't add memory device [0x%" PRIx64 ":0x%" PRIx64
>> +                       "] before 0x%" PRIx64, range_lob(&new), range_size(&new),
>> +                       range_lob(&as));
>> +            return 0;
>> +        } else if (range_ends_after_range(&new, &as)) {
>>              error_setg(errp, "can't add memory device [0x%" PRIx64 ":0x%" PRIx64
>> -                       "] beyond 0x%" PRIx64, new_addr, size,
>> -                       address_space_end);
>> +                       "] beyond 0x%" PRIx64, range_lob(&new), range_size(&new),
>> +                       range_upb(&as));
> 
> Would !range_contains_range be easier here?

Yes, but then we have to change the error message, e.g. to

can't add memory device [...:...], usable range [...:...]

> 
>>              return 0;
>>          }
>>      } else {
>> -        new_addr = address_space_start;
>> +        /* our previous size checks make sure that this never overflows */
> 
> Hmm, which one exactly?

Previous patch in this series, via memory_device_check_addable().

But we can add an explicit check here.

> 
>> +        range_init(&new, range_lob(&as), size);
>>      }



-- 

Thanks,

David / dhildenb