Re: [Qemu-devel] [PATCH] pcie: remove duplicate assertion

[Cao jin]

On 01/10/2017 06:37 AM, Michael S. Tsirkin wrote:
> On Fri, Dec 23, 2016 at 10:16:30AM +0800, Cao jin wrote:
>> "size >= 8" connote "size > 0"
>>
>> Signed-off-by: Cao jin 
> 
> Isn't the point to check for overflows?
> 

Make sense. If it is intended to check overflows, the following sequence
would make more sense:

assert(offset >= PCI_CONFIG_SPACE_SIZE);
assert(size >= 8);
assert(offset < offset + size);
assert(offset + size <= PCIE_CONFIG_SPACE_SIZE);

or else, size 0 will pass the assert(offset < offset + size) first and
hit assert(size >= 8)
-- 
Sincerely,
Cao jin

>> ---
>>  hw/pci/pcie.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
>> index 39b10b852d91..f864c5cd5458 100644
>> --- a/hw/pci/pcie.c
>> +++ b/hw/pci/pcie.c
>> @@ -668,7 +668,6 @@ void pcie_add_capability(PCIDevice *dev,
>>  uint16_t next;
>>  
>>  assert(offset >= PCI_CONFIG_SPACE_SIZE);
>> -assert(offset < offset + size);
>>  assert(offset + size <= PCIE_CONFIG_SPACE_SIZE);
>>  assert(size >= 8);
>>  assert(pci_is_express(dev));
>> -- 
>> 2.1.0
>>
>>
> 
> 
> 

Re: [Qemu-devel] [PATCH] pcie: remove duplicate assertion

[Michael S. Tsirkin]

On Fri, Dec 23, 2016 at 10:16:30AM +0800, Cao jin wrote:
> "size >= 8" connote "size > 0"
> 
> Signed-off-by: Cao jin 

Isn't the point to check for overflows?

> ---
>  hw/pci/pcie.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 39b10b852d91..f864c5cd5458 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -668,7 +668,6 @@ void pcie_add_capability(PCIDevice *dev,
>  uint16_t next;
>  
>  assert(offset >= PCI_CONFIG_SPACE_SIZE);
> -assert(offset < offset + size);
>  assert(offset + size <= PCIE_CONFIG_SPACE_SIZE);
>  assert(size >= 8);
>  assert(pci_is_express(dev));
> -- 
> 2.1.0
> 
>