* [Qemu-devel] [PATCH] Coverity Fix
@ 2016-10-01 15:57 David Kiarie
2016-10-01 15:57 ` [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan David Kiarie
0 siblings, 1 reply; 5+ messages in thread
From: David Kiarie @ 2016-10-01 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jan.kiszka, mst, peterx, pbonzini, David Kiarie
Hi all,
The following patch fixes a few issues reported by coverity in the file hw/i386/amd_iommu.c
David Kiarie (1):
hw/iommu: Fix problems reported by Coverity scan
hw/i386/amd_iommu.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan
2016-10-01 15:57 [Qemu-devel] [PATCH] Coverity Fix David Kiarie
@ 2016-10-01 15:57 ` David Kiarie
2016-10-01 16:29 ` Stefan Weil
0 siblings, 1 reply; 5+ messages in thread
From: David Kiarie @ 2016-10-01 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, jan.kiszka, mst, peterx, pbonzini, David Kiarie
Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
---
hw/i386/amd_iommu.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 023de52..815d45f 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -144,7 +144,7 @@ static void amdvi_assign_andq(AMDVIState *s, hwaddr addr, uint64_t val)
static void amdvi_generate_msi_interrupt(AMDVIState *s)
{
MSIMessage msg;
- MemTxAttrs attrs;
+ MemTxAttrs attrs = {0, 0};
attrs.requester_id = pci_requester_id(&s->pci.dev);
@@ -185,7 +185,7 @@ static void amdvi_setevent_bits(uint64_t *buffer, uint64_t value, int start,
int length)
{
int index = start / 64, bitpos = start % 64;
- uint64_t mask = ((1 << length) - 1) << bitpos;
+ uint64_t mask = ((1UL << length) - 1) << bitpos;
buffer[index] &= ~mask;
buffer[index] |= (value << bitpos) & mask;
}
@@ -334,7 +334,7 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t devid,
uint16_t domid)
{
AMDVIIOTLBEntry *entry = g_malloc(sizeof(*entry));
- uint64_t *key = g_malloc(sizeof(key));
+ uint64_t *key = g_malloc(sizeof(*key));
uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
/* don't cache erroneous translations */
@@ -1135,6 +1135,7 @@ static void amdvi_reset(DeviceState *dev)
static void amdvi_realize(DeviceState *dev, Error **err)
{
+ int ret = 0;
AMDVIState *s = AMD_IOMMU_DEVICE(dev);
X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus;
@@ -1147,8 +1148,11 @@ static void amdvi_realize(DeviceState *dev, Error **err)
object_property_set_bool(OBJECT(&s->pci), true, "realized", err);
s->capab_offset = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
AMDVI_CAPAB_SIZE);
- pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0, AMDVI_CAPAB_REG_SIZE);
+ assert(s->capab_offset > 0);
+ ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0, AMDVI_CAPAB_REG_SIZE);
+ assert(ret > 0);
pci_add_capability(&s->pci.dev, PCI_CAP_ID_HT, 0, AMDVI_CAPAB_REG_SIZE);
+ assert(ret > 0);
/* set up MMIO */
memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan
2016-10-01 15:57 ` [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan David Kiarie
@ 2016-10-01 16:29 ` Stefan Weil
2016-10-01 16:53 ` David Kiarie
2016-10-04 8:06 ` Markus Armbruster
0 siblings, 2 replies; 5+ messages in thread
From: Stefan Weil @ 2016-10-01 16:29 UTC (permalink / raw)
To: David Kiarie, qemu-devel; +Cc: peter.maydell, mst, peterx, jan.kiszka, pbonzini
Hi,
On 10/01/16 17:57, David Kiarie wrote:
> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
> ---
> hw/i386/amd_iommu.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 023de52..815d45f 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -144,7 +144,7 @@ static void amdvi_assign_andq(AMDVIState *s, hwaddr addr, uint64_t val)
> static void amdvi_generate_msi_interrupt(AMDVIState *s)
> {
> MSIMessage msg;
> - MemTxAttrs attrs;
> + MemTxAttrs attrs = {0, 0};
>
> attrs.requester_id = pci_requester_id(&s->pci.dev);
>
> @@ -185,7 +185,7 @@ static void amdvi_setevent_bits(uint64_t *buffer, uint64_t value, int start,
> int length)
> {
> int index = start / 64, bitpos = start % 64;
> - uint64_t mask = ((1 << length) - 1) << bitpos;
> + uint64_t mask = ((1UL << length) - 1) << bitpos;
> buffer[index] &= ~mask;
> buffer[index] |= (value << bitpos) & mask;
> }
> @@ -334,7 +334,7 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t devid,
> uint16_t domid)
> {
> AMDVIIOTLBEntry *entry = g_malloc(sizeof(*entry));
> - uint64_t *key = g_malloc(sizeof(key));
> + uint64_t *key = g_malloc(sizeof(*key));
I suggest using g_new(uint64_t, 1) here.
> uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
>
> /* don't cache erroneous translations */
> @@ -1135,6 +1135,7 @@ static void amdvi_reset(DeviceState *dev)
>
> static void amdvi_realize(DeviceState *dev, Error **err)
> {
> + int ret = 0;
> AMDVIState *s = AMD_IOMMU_DEVICE(dev);
> X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
> PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus;
> @@ -1147,8 +1148,11 @@ static void amdvi_realize(DeviceState *dev, Error **err)
> object_property_set_bool(OBJECT(&s->pci), true, "realized", err);
> s->capab_offset = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
> AMDVI_CAPAB_SIZE);
> - pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0, AMDVI_CAPAB_REG_SIZE);
> + assert(s->capab_offset > 0);
> + ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0, AMDVI_CAPAB_REG_SIZE);
> + assert(ret > 0);
> pci_add_capability(&s->pci.dev, PCI_CAP_ID_HT, 0, AMDVI_CAPAB_REG_SIZE);
Did you forget to assign the result to ret here? Without that, the
following assertion does not make sense, and coverity will still complain.
> + assert(ret > 0);
>
> /* set up MMIO */
> memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
>
Stefan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan
2016-10-01 16:29 ` Stefan Weil
@ 2016-10-01 16:53 ` David Kiarie
2016-10-04 8:06 ` Markus Armbruster
1 sibling, 0 replies; 5+ messages in thread
From: David Kiarie @ 2016-10-01 16:53 UTC (permalink / raw)
To: Stefan Weil
Cc: QEMU Developers, Peter Maydell, Michael S. Tsirkin, Peter Xu,
Jan Kiszka, Paolo Bonzini
On Sat, Oct 1, 2016 at 7:29 PM, Stefan Weil <sw@weilnetz.de> wrote:
> Hi,
>
>
> On 10/01/16 17:57, David Kiarie wrote:
>>
>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
>> ---
>> hw/i386/amd_iommu.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 023de52..815d45f 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -144,7 +144,7 @@ static void amdvi_assign_andq(AMDVIState *s, hwaddr
>> addr, uint64_t val)
>> static void amdvi_generate_msi_interrupt(AMDVIState *s)
>> {
>> MSIMessage msg;
>> - MemTxAttrs attrs;
>> + MemTxAttrs attrs = {0, 0};
>>
>> attrs.requester_id = pci_requester_id(&s->pci.dev);
>>
>> @@ -185,7 +185,7 @@ static void amdvi_setevent_bits(uint64_t *buffer,
>> uint64_t value, int start,
>> int length)
>> {
>> int index = start / 64, bitpos = start % 64;
>> - uint64_t mask = ((1 << length) - 1) << bitpos;
>> + uint64_t mask = ((1UL << length) - 1) << bitpos;
>> buffer[index] &= ~mask;
>> buffer[index] |= (value << bitpos) & mask;
>> }
>> @@ -334,7 +334,7 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t
>> devid,
>> uint16_t domid)
>> {
>> AMDVIIOTLBEntry *entry = g_malloc(sizeof(*entry));
>> - uint64_t *key = g_malloc(sizeof(key));
>> + uint64_t *key = g_malloc(sizeof(*key));
>
>
> I suggest using g_new(uint64_t, 1) here.
>
>> uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
>>
>> /* don't cache erroneous translations */
>> @@ -1135,6 +1135,7 @@ static void amdvi_reset(DeviceState *dev)
>>
>> static void amdvi_realize(DeviceState *dev, Error **err)
>> {
>> + int ret = 0;
>> AMDVIState *s = AMD_IOMMU_DEVICE(dev);
>> X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>> PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus;
>> @@ -1147,8 +1148,11 @@ static void amdvi_realize(DeviceState *dev, Error
>> **err)
>> object_property_set_bool(OBJECT(&s->pci), true, "realized", err);
>> s->capab_offset = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC,
>> 0,
>> AMDVI_CAPAB_SIZE);
>> - pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0,
>> AMDVI_CAPAB_REG_SIZE);
>> + assert(s->capab_offset > 0);
>> + ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0,
>> AMDVI_CAPAB_REG_SIZE);
>> + assert(ret > 0);
>> pci_add_capability(&s->pci.dev, PCI_CAP_ID_HT, 0,
>> AMDVI_CAPAB_REG_SIZE);
>
>
> Did you forget to assign the result to ret here? Without that, the following
> assertion does not make sense, and coverity will still complain.
Yeah, missed something here.
>
>
>> + assert(ret > 0);
>>
>> /* set up MMIO */
>> memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s,
>> "amdvi-mmio",
>>
>
> Stefan
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan
2016-10-01 16:29 ` Stefan Weil
2016-10-01 16:53 ` David Kiarie
@ 2016-10-04 8:06 ` Markus Armbruster
1 sibling, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2016-10-04 8:06 UTC (permalink / raw)
To: Stefan Weil
Cc: David Kiarie, qemu-devel, peter.maydell, pbonzini, jan.kiszka,
peterx, mst
Stefan Weil <sw@weilnetz.de> writes:
> Hi,
>
> On 10/01/16 17:57, David Kiarie wrote:
>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
>> ---
>> hw/i386/amd_iommu.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 023de52..815d45f 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -144,7 +144,7 @@ static void amdvi_assign_andq(AMDVIState *s, hwaddr addr, uint64_t val)
>> static void amdvi_generate_msi_interrupt(AMDVIState *s)
>> {
>> MSIMessage msg;
>> - MemTxAttrs attrs;
>> + MemTxAttrs attrs = {0, 0};
>>
>> attrs.requester_id = pci_requester_id(&s->pci.dev);
>>
>> @@ -185,7 +185,7 @@ static void amdvi_setevent_bits(uint64_t *buffer, uint64_t value, int start,
>> int length)
>> {
>> int index = start / 64, bitpos = start % 64;
>> - uint64_t mask = ((1 << length) - 1) << bitpos;
>> + uint64_t mask = ((1UL << length) - 1) << bitpos;
>> buffer[index] &= ~mask;
>> buffer[index] |= (value << bitpos) & mask;
>> }
>> @@ -334,7 +334,7 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t devid,
>> uint16_t domid)
>> {
>> AMDVIIOTLBEntry *entry = g_malloc(sizeof(*entry));
>> - uint64_t *key = g_malloc(sizeof(key));
>> + uint64_t *key = g_malloc(sizeof(*key));
>
> I suggest using g_new(uint64_t, 1) here.
Either way is fine.
g_new(T, 1) is clearly superior to g_malloc(sizeof(T)), because it's
terser, and yields a more useful type.
v = g_new(T, 1) vs. v = g_malloc(sizeof(*v)) is less clear. The g_new()
is more tightly typed, but the typing doesn't buy much here. The
g_malloc() is idiomatic usage. Matter of taste.
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-04 8:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-01 15:57 [Qemu-devel] [PATCH] Coverity Fix David Kiarie
2016-10-01 15:57 ` [Qemu-devel] [PATCH] hw/iommu: Fix problems reported by Coverity scan David Kiarie
2016-10-01 16:29 ` Stefan Weil
2016-10-01 16:53 ` David Kiarie
2016-10-04 8:06 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.