[Qemu-devel] [PATCH 0/2] qemu-ga: Plug a few leaks

[Qemu-devel] [PATCH 0/2] qemu-ga: Plug a few leaks

[Markus Armbruster]

Markus Armbruster (2):
  qemu-ga: Plug memory leak in guest_fsfreeze_cleanup()
  qemu-ga: Plug leaks on qmp_guest_network_get_interfaces() error paths

 qga/commands-posix.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH 1/2] qemu-ga: Plug memory leak in guest_fsfreeze_cleanup()

[Markus Armbruster]

Neglects to free errors allocated by qmp_guest_fsfreeze_thaw().
Spotted by Coverity.

While there, drop the test whether return value is negative (it's
never true), and improve logging.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/commands-posix.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 77f6ee7..f95ebc8 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -611,13 +611,14 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
 
 static void guest_fsfreeze_cleanup(void)
 {
-    int64_t ret;
     Error *err = NULL;
 
     if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) {
-        ret = qmp_guest_fsfreeze_thaw(&err);
-        if (ret < 0 || err) {
-            slog("failed to clean up frozen filesystems");
+        qmp_guest_fsfreeze_thaw(&err);
+        if (err) {
+            slog("failed to clean up frozen filesystems: %s",
+                 error_get_pretty(err));
+            error_free(err);
         }
     }
 }
-- 
1.7.11.7

[Qemu-devel] [PATCH 2/2] qemu-ga: Plug leaks on qmp_guest_network_get_interfaces() error paths

[Markus Armbruster]

Spotted by Coverity.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/commands-posix.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f95ebc8..d6902a9 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -935,9 +935,11 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
                 error_setg_errno(errp, errno,
                                  "failed to get MAC address of %s",
                                  ifa->ifa_name);
+                close(sock);
                 goto error;
             }
 
+            close(sock);
             mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
 
             if (asprintf(&info->value->hardware_address,
@@ -950,20 +952,19 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
             }
 
             info->value->has_hardware_address = true;
-            close(sock);
         }
 
         if (ifa->ifa_addr &&
             ifa->ifa_addr->sa_family == AF_INET) {
             /* interface with IPv4 address */
-            address_item = g_malloc0(sizeof(*address_item));
-            address_item->value = g_malloc0(sizeof(*address_item->value));
             p = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
             if (!inet_ntop(AF_INET, p, addr4, sizeof(addr4))) {
                 error_setg_errno(errp, errno, "inet_ntop failed");
                 goto error;
             }
 
+            address_item = g_malloc0(sizeof(*address_item));
+            address_item->value = g_malloc0(sizeof(*address_item->value));
             address_item->value->ip_address = g_strdup(addr4);
             address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
 
@@ -976,14 +977,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
         } else if (ifa->ifa_addr &&
                    ifa->ifa_addr->sa_family == AF_INET6) {
             /* interface with IPv6 address */
-            address_item = g_malloc0(sizeof(*address_item));
-            address_item->value = g_malloc0(sizeof(*address_item->value));
             p = &((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr;
             if (!inet_ntop(AF_INET6, p, addr6, sizeof(addr6))) {
                 error_setg_errno(errp, errno, "inet_ntop failed");
                 goto error;
             }
 
+            address_item = g_malloc0(sizeof(*address_item));
+            address_item->value = g_malloc0(sizeof(*address_item->value));
             address_item->value->ip_address = g_strdup(addr6);
             address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
 
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 0/2] qemu-ga: Plug a few leaks

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 484 bytes --]

On 01/16/2013 10:15 AM, Markus Armbruster wrote:
> Markus Armbruster (2):
>   qemu-ga: Plug memory leak in guest_fsfreeze_cleanup()
>   qemu-ga: Plug leaks on qmp_guest_network_get_interfaces() error paths
> 
>  qga/commands-posix.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

Series: Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 2/2] qemu-ga: Plug leaks on qmp_guest_network_get_interfaces() error paths

[Luiz Capitulino]

On Wed, 16 Jan 2013 18:15:09 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Spotted by Coverity.

I'd split that patch, but you don't have to respin just because of that.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/commands-posix.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index f95ebc8..d6902a9 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -935,9 +935,11 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>                  error_setg_errno(errp, errno,
>                                   "failed to get MAC address of %s",
>                                   ifa->ifa_name);
> +                close(sock);
>                  goto error;
>              }
>  
> +            close(sock);
>              mac_addr = (unsigned char *) &ifr.ifr_hwaddr.sa_data;
>  
>              if (asprintf(&info->value->hardware_address,
> @@ -950,20 +952,19 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>              }
>  
>              info->value->has_hardware_address = true;
> -            close(sock);
>          }
>  
>          if (ifa->ifa_addr &&
>              ifa->ifa_addr->sa_family == AF_INET) {
>              /* interface with IPv4 address */
> -            address_item = g_malloc0(sizeof(*address_item));
> -            address_item->value = g_malloc0(sizeof(*address_item->value));
>              p = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
>              if (!inet_ntop(AF_INET, p, addr4, sizeof(addr4))) {
>                  error_setg_errno(errp, errno, "inet_ntop failed");
>                  goto error;
>              }
>  
> +            address_item = g_malloc0(sizeof(*address_item));
> +            address_item->value = g_malloc0(sizeof(*address_item->value));
>              address_item->value->ip_address = g_strdup(addr4);
>              address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV4;
>  
> @@ -976,14 +977,14 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>          } else if (ifa->ifa_addr &&
>                     ifa->ifa_addr->sa_family == AF_INET6) {
>              /* interface with IPv6 address */
> -            address_item = g_malloc0(sizeof(*address_item));
> -            address_item->value = g_malloc0(sizeof(*address_item->value));
>              p = &((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr;
>              if (!inet_ntop(AF_INET6, p, addr6, sizeof(addr6))) {
>                  error_setg_errno(errp, errno, "inet_ntop failed");
>                  goto error;
>              }
>  
> +            address_item = g_malloc0(sizeof(*address_item));
> +            address_item->value = g_malloc0(sizeof(*address_item->value));
>              address_item->value->ip_address = g_strdup(addr6);
>              address_item->value->ip_address_type = GUEST_IP_ADDRESS_TYPE_IPV6;
>  

Re: [Qemu-devel] [PATCH 0/2] qemu-ga: Plug a few leaks

[Luiz Capitulino]

On Wed, 16 Jan 2013 18:15:07 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Markus Armbruster (2):
>   qemu-ga: Plug memory leak in guest_fsfreeze_cleanup()
>   qemu-ga: Plug leaks on qmp_guest_network_get_interfaces() error paths

Nice ones. Series:

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> 
>  qga/commands-posix.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>