diff mbox

[v5,10/10] msi_init: convert assert to return -errno

Message ID 1478153400-29946-1-git-send-email-caoj.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

Cao jin Nov. 3, 2016, 6:10 a.m. UTC
According to the disscussion:
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html

Let leaf function returns reasonable -errno, let caller decide how to
handle the return value.

Suggested-by: Markus Armbruster <armbru@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Marcel Apfelbaum <marcel@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
---
 hw/pci/msi.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Really sorry, I forget to commit what I amend...

Comments

Marcel Apfelbaum Nov. 3, 2016, 11:47 a.m. UTC | #1
On 11/03/2016 08:10 AM, Cao jin wrote:
> According to the disscussion:
> http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html
>

Hi,

Intense discussion, but I learned a lot from it.

> Let leaf function returns reasonable -errno, let caller decide how to
> handle the return value.
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Marcel Apfelbaum <marcel@redhat.com>
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> ---
>  hw/pci/msi.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> Really sorry, I forget to commit what I amend...
>
> diff --git a/hw/pci/msi.c b/hw/pci/msi.c
> index a87b227..af3efbe 100644
> --- a/hw/pci/msi.c
> +++ b/hw/pci/msi.c
> @@ -201,9 +201,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
>                     " 64bit %d mask %d\n",
>                     offset, nr_vectors, msi64bit, msi_per_vector_mask);
>
> -    assert(!(nr_vectors & (nr_vectors - 1)));   /* power of 2 */
> -    assert(nr_vectors > 0);
> -    assert(nr_vectors <= PCI_MSI_VECTORS_MAX);
> +    /* vector sanity test: should in range 1 - 32, should be power of 2 */
> +    if (!is_power_of_2(nr_vectors) || nr_vectors > PCI_MSI_VECTORS_MAX) {
> +        error_setg(errp, "Invalid vector number: %d", nr_vectors);
> +        return -EINVAL;
> +    }
> +
>      /* the nr of MSI vectors is up to 32 */
>      vectors_order = ctz32(nr_vectors);
>
>


Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>


Thanks,
Marcel
diff mbox

Patch

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..af3efbe 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -201,9 +201,12 @@  int msi_init(struct PCIDevice *dev, uint8_t offset,
                    " 64bit %d mask %d\n",
                    offset, nr_vectors, msi64bit, msi_per_vector_mask);
 
-    assert(!(nr_vectors & (nr_vectors - 1)));   /* power of 2 */
-    assert(nr_vectors > 0);
-    assert(nr_vectors <= PCI_MSI_VECTORS_MAX);
+    /* vector sanity test: should in range 1 - 32, should be power of 2 */
+    if (!is_power_of_2(nr_vectors) || nr_vectors > PCI_MSI_VECTORS_MAX) {
+        error_setg(errp, "Invalid vector number: %d", nr_vectors);
+        return -EINVAL;
+    }
+
     /* the nr of MSI vectors is up to 32 */
     vectors_order = ctz32(nr_vectors);