[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/6] cloop: Fix bdrv_open() error handling
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 2/6] cloop: Fix bdrv_open() error handling |
Date: |
Thu, 24 Jan 2013 13:48:45 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Return -errno instead of -1 on errors. While touching the
> code, fix a memory leak.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block/cloop.c | 27 +++++++++++++++++----------
> 1 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/block/cloop.c b/block/cloop.c
> index 5a0d0d8..9b36063 100644
> --- a/block/cloop.c
> +++ b/block/cloop.c
> @@ -57,27 +57,32 @@ static int cloop_open(BlockDriverState *bs, int flags)
> {
> BDRVCloopState *s = bs->opaque;
> uint32_t offsets_size, max_compressed_block_size = 1, i;
> + int ret;
>
> bs->read_only = 1;
>
> /* read header */
> - if (bdrv_pread(bs->file, 128, &s->block_size, 4) < 4) {
> - goto cloop_close;
> + ret = bdrv_pread(bs->file, 128, &s->block_size, 4);
> + if (ret < 0) {
> + return ret;
> }
> s->block_size = be32_to_cpu(s->block_size);
>
> - if (bdrv_pread(bs->file, 128 + 4, &s->n_blocks, 4) < 4) {
> - goto cloop_close;
> + ret = bdrv_pread(bs->file, 128 + 4, &s->n_blocks, 4);
> + if (ret < 0) {
> + return ret;
> }
> s->n_blocks = be32_to_cpu(s->n_blocks);
>
> /* read offsets */
> offsets_size = s->n_blocks * sizeof(uint64_t);
> s->offsets = g_malloc(offsets_size);
> - if (bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size) <
> - offsets_size) {
> - goto cloop_close;
> +
Empty line visually detaches the /* read offsets */ comment from the
actual read. Sure you want it?
> + ret = bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size);
> + if (ret < 0) {
> + goto fail;
> }
> +
> for(i=0;i<s->n_blocks;i++) {
> s->offsets[i] = be64_to_cpu(s->offsets[i]);
> if (i > 0) {
> @@ -92,7 +97,8 @@ static int cloop_open(BlockDriverState *bs, int flags)
> s->compressed_block = g_malloc(max_compressed_block_size + 1);
> s->uncompressed_block = g_malloc(s->block_size);
> if (inflateInit(&s->zstream) != Z_OK) {
> - goto cloop_close;
> + ret = -EINVAL;
inflateInit() can return a number of different errors. But your change
doesn't make things worse, and that's good enough.
> + goto fail;
> }
> s->current_block = s->n_blocks;
>
> @@ -101,8 +107,9 @@ static int cloop_open(BlockDriverState *bs, int flags)
> qemu_co_mutex_init(&s->lock);
> return 0;
>
> -cloop_close:
> - return -1;
> +fail:
> + g_free(s->offsets);
What about s->compressed_block and s->uncompressed_block?
> + return ret;
> }
>
> static inline int cloop_read_block(BlockDriverState *bs, int block_num)
[Qemu-devel] [PATCH 5/6] dmg: Use g_free instead of free, Kevin Wolf, 2013/01/24