Libguestfs - Nov 2017 - [PATCH] daemon: btrfs: fix bad handling of a couple of errors

Remove two wrong error handling situations in btrfs_balance_status:
- if the calloc for 'ret' fails, the 'goto error' would try to
  dereference the resulting null pointer (to free the items of the
  return struct); hence, just return null directly, instead of jumping
  to 'error'
- if the strdup() for 'btrfsbalance_status' fails, then the directly
  return would leak 'ret'
---
 daemon/btrfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index d363ada6d..26757d4fb 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -1686,7 +1686,7 @@ do_btrfs_balance_status (const char *path)
   ret = calloc (1, sizeof *ret);
   if (ret == NULL) {
     reply_with_perror ("calloc");
-    goto error;
+    return NULL;
   }
 
   /* Output of `btrfs balance status' is like:
@@ -1715,7 +1715,7 @@ do_btrfs_balance_status (const char *path)
     ret->btrfsbalance_status = strdup ("none");
     if (ret->btrfsbalance_status == NULL) {
       reply_with_perror ("strdup");
-      return NULL;
+      goto error;
     }
     return ret;
   }
-- 
2.13.6

On Fri, Nov 03, 2017 at 05:31:21PM +0100, Pino Toscano
wrote:
> Remove two wrong error handling situations in btrfs_balance_status:
> - if the calloc for 'ret' fails, the 'goto error' would try
to
>   dereference the resulting null pointer (to free the items of the
>   return struct); hence, just return null directly, instead of jumping
>   to 'error'
> - if the strdup() for 'btrfsbalance_status' fails, then the
directly
>   return would leak 'ret'
> ---
>  daemon/btrfs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index d363ada6d..26757d4fb 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -1686,7 +1686,7 @@ do_btrfs_balance_status (const char *path)
>    ret = calloc (1, sizeof *ret);
>    if (ret == NULL) {
>      reply_with_perror ("calloc");
> -    goto error;
> +    return NULL;
>    }
>  
>    /* Output of `btrfs balance status' is like:
> @@ -1715,7 +1715,7 @@ do_btrfs_balance_status (const char *path)
>      ret->btrfsbalance_status = strdup ("none");
>      if (ret->btrfsbalance_status == NULL) {
>        reply_with_perror ("strdup");
> -      return NULL;
> +      goto error;
>      }
>      return ret;
>    }
> -- 
> 2.13.6
ACK.

These are from Coverity right?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top