diff mbox

[v2,1/4] acpi/pcihp.c: Rewrite acpi_pcihp_get_bsel using object_property_get_int

Message ID 1398348959-23048-2-git-send-email-batuzovk@ispras.ru
State New
Headers show

Commit Message

Kirill Batuzov April 24, 2014, 2:15 p.m. UTC
acpi_pcihp_get_bsel implements functionality of object_property_get_int for
specific property named ACPI_PCIHP_PROP_BSEL, but fails to decrement object's
reference counter properly. Rewriting it using generic object_property_get_int
serves two purposes: reducing code duplication and fixing memory leak.

Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
---
 hw/acpi/pcihp.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

v1 -> v2:
 Keep acpi_pcihp_get_bsel, but rewrite it using object_property_get_int and
 validate returned value.

Comments

Michael S. Tsirkin April 24, 2014, 2:19 p.m. UTC | #1
On Thu, Apr 24, 2014 at 06:15:56PM +0400, Kirill Batuzov wrote:
> acpi_pcihp_get_bsel implements functionality of object_property_get_int for
> specific property named ACPI_PCIHP_PROP_BSEL, but fails to decrement object's
> reference counter properly. Rewriting it using generic object_property_get_int
> serves two purposes: reducing code duplication and fixing memory leak.
> 
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


which tree would you like to merge this through? mine?


> ---
>  hw/acpi/pcihp.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> v1 -> v2:
>  Keep acpi_pcihp_get_bsel, but rewrite it using object_property_get_int and
>  validate returned value.
> 
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index f80c480..3b143b3 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -63,16 +63,18 @@ typedef struct AcpiPciHpFind {
>  
>  static int acpi_pcihp_get_bsel(PCIBus *bus)
>  {
> -    QObject *o = object_property_get_qobject(OBJECT(bus),
> -                                             ACPI_PCIHP_PROP_BSEL, NULL);
> -    int64_t bsel = -1;
> -    if (o) {
> -        bsel = qint_get_int(qobject_to_qint(o));
> -    }
> -    if (bsel < 0) {
> +    Error *local_err = NULL;
> +    int64_t bsel = object_property_get_int(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
> +                                           &local_err);
> +
> +    if (local_err || bsel < 0 || bsel >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
> +        if (local_err) {
> +            error_free(local_err);
> +        }
>          return -1;
> +    } else {
> +        return bsel;
>      }
> -    return bsel;
>  }
>  
>  static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque)
> -- 
> 1.7.10.4
Michael S. Tsirkin April 24, 2014, 3:37 p.m. UTC | #2
On Thu, Apr 24, 2014 at 06:15:56PM +0400, Kirill Batuzov wrote:
> acpi_pcihp_get_bsel implements functionality of object_property_get_int for
> specific property named ACPI_PCIHP_PROP_BSEL, but fails to decrement object's
> reference counter properly. Rewriting it using generic object_property_get_int
> serves two purposes: reducing code duplication and fixing memory leak.
> 
> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>

Applied this and 2/4. Thanks!

> ---
>  hw/acpi/pcihp.c |   18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> v1 -> v2:
>  Keep acpi_pcihp_get_bsel, but rewrite it using object_property_get_int and
>  validate returned value.
> 
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index f80c480..3b143b3 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -63,16 +63,18 @@ typedef struct AcpiPciHpFind {
>  
>  static int acpi_pcihp_get_bsel(PCIBus *bus)
>  {
> -    QObject *o = object_property_get_qobject(OBJECT(bus),
> -                                             ACPI_PCIHP_PROP_BSEL, NULL);
> -    int64_t bsel = -1;
> -    if (o) {
> -        bsel = qint_get_int(qobject_to_qint(o));
> -    }
> -    if (bsel < 0) {
> +    Error *local_err = NULL;
> +    int64_t bsel = object_property_get_int(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
> +                                           &local_err);
> +
> +    if (local_err || bsel < 0 || bsel >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
> +        if (local_err) {
> +            error_free(local_err);
> +        }
>          return -1;
> +    } else {
> +        return bsel;
>      }
> -    return bsel;
>  }
>  
>  static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque)
> -- 
> 1.7.10.4
diff mbox

Patch

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index f80c480..3b143b3 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -63,16 +63,18 @@  typedef struct AcpiPciHpFind {
 
 static int acpi_pcihp_get_bsel(PCIBus *bus)
 {
-    QObject *o = object_property_get_qobject(OBJECT(bus),
-                                             ACPI_PCIHP_PROP_BSEL, NULL);
-    int64_t bsel = -1;
-    if (o) {
-        bsel = qint_get_int(qobject_to_qint(o));
-    }
-    if (bsel < 0) {
+    Error *local_err = NULL;
+    int64_t bsel = object_property_get_int(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
+                                           &local_err);
+
+    if (local_err || bsel < 0 || bsel >= ACPI_PCIHP_MAX_HOTPLUG_BUS) {
+        if (local_err) {
+            error_free(local_err);
+        }
         return -1;
+    } else {
+        return bsel;
     }
-    return bsel;
 }
 
 static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque)