qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] use g_free, instead of free
@ 2011-11-01  6:21 Dong Xu Wang
  2011-11-01  7:54 ` Stefan Hajnoczi
  2011-11-01  8:41 ` Andreas Färber
  0 siblings, 2 replies; 3+ messages in thread
From: Dong Xu Wang @ 2011-11-01  6:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Dong Xu Wang, stefanha

From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>

Fix mismatching allocation and deallocation: g_free should be used to pair with g_malloc.
Also fix coding style.

Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
---
 block/cloop.c |  119 +++++++++++++++++++++++++++++++--------------------------
 1 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/block/cloop.c b/block/cloop.c
index 775f8a9..1884b8c 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -30,7 +30,7 @@ typedef struct BDRVCloopState {
     CoMutex lock;
     uint32_t block_size;
     uint32_t n_blocks;
-    uint64_t* offsets;
+    uint64_t *offsets;
     uint32_t sectors_per_block;
     uint32_t current_block;
     uint8_t *compressed_block;
@@ -40,21 +40,23 @@ typedef struct BDRVCloopState {
 
 static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
-    const char* magic_version_2_0="#!/bin/sh\n"
-	"#V2.0 Format\n"
-	"modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n";
-    int length=strlen(magic_version_2_0);
-    if(length>buf_size)
-	length=buf_size;
-    if(!memcmp(magic_version_2_0,buf,length))
-	return 2;
+    const char *magic_version_2_0 = "#!/bin/sh\n"
+        "#V2.0 Format\n"
+        "modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n";
+    int length = strlen(magic_version_2_0);
+    if (length > buf_size) {
+        length = buf_size;
+    }
+    if (!memcmp(magic_version_2_0, buf, length)) {
+        return 2;
+    }
     return 0;
 }
 
 static int cloop_open(BlockDriverState *bs, int flags)
 {
     BDRVCloopState *s = bs->opaque;
-    uint32_t offsets_size,max_compressed_block_size=1,i;
+    uint32_t offsets_size, max_compressed_block_size = 1, i;
 
     bs->read_only = 1;
 
@@ -74,26 +76,28 @@ static int cloop_open(BlockDriverState *bs, int flags)
     s->offsets = g_malloc(offsets_size);
     if (bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size) <
             offsets_size) {
-	goto cloop_close;
+        goto cloop_close;
     }
     for(i=0;i<s->n_blocks;i++) {
-	s->offsets[i]=be64_to_cpu(s->offsets[i]);
-	if(i>0) {
-	    uint32_t size=s->offsets[i]-s->offsets[i-1];
-	    if(size>max_compressed_block_size)
-		max_compressed_block_size=size;
-	}
+        s->offsets[i] = be64_to_cpu(s->offsets[i]);
+        if (i > 0) {
+            uint32_t size = s->offsets[i] - s->offsets[i-1];
+            if (size > max_compressed_block_size) {
+                max_compressed_block_size = size;
+            }
+        }
     }
 
     /* initialize zlib engine */
-    s->compressed_block = g_malloc(max_compressed_block_size+1);
+    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;
-    s->current_block=s->n_blocks;
+    if (inflateInit(&s->zstream) != Z_OK) {
+        goto cloop_close;
+    }
+    s->current_block = s->n_blocks;
 
     s->sectors_per_block = s->block_size/512;
-    bs->total_sectors = s->n_blocks*s->sectors_per_block;
+    bs->total_sectors = s->n_blocks * s->sectors_per_block;
     qemu_co_mutex_init(&s->lock);
     return 0;
 
@@ -105,27 +109,30 @@ static inline int cloop_read_block(BlockDriverState *bs, int block_num)
 {
     BDRVCloopState *s = bs->opaque;
 
-    if(s->current_block != block_num) {
-	int ret;
-        uint32_t bytes = s->offsets[block_num+1]-s->offsets[block_num];
+    if (s->current_block != block_num) {
+        int ret;
+        uint32_t bytes = s->offsets[block_num + 1]-s->offsets[block_num];
 
         ret = bdrv_pread(bs->file, s->offsets[block_num], s->compressed_block,
                          bytes);
-        if (ret != bytes)
+        if (ret != bytes) {
             return -1;
+        }
+
+        s->zstream.next_in = s->compressed_block;
+        s->zstream.avail_in = bytes;
+        s->zstream.next_out = s->uncompressed_block;
+        s->zstream.avail_out = s->block_size;
+        ret = inflateReset(&s->zstream);
+        if (ret != Z_OK) {
+            return -1;
+        }
+        ret = inflate(&s->zstream, Z_FINISH);
+        if (ret != Z_STREAM_END || s->zstream.total_out != s->block_size) {
+            return -1;
+        }
 
-	s->zstream.next_in = s->compressed_block;
-	s->zstream.avail_in = bytes;
-	s->zstream.next_out = s->uncompressed_block;
-	s->zstream.avail_out = s->block_size;
-	ret = inflateReset(&s->zstream);
-	if(ret != Z_OK)
-	    return -1;
-	ret = inflate(&s->zstream, Z_FINISH);
-	if(ret != Z_STREAM_END || s->zstream.total_out != s->block_size)
-	    return -1;
-
-	s->current_block = block_num;
+        s->current_block = block_num;
     }
     return 0;
 }
@@ -136,12 +143,15 @@ static int cloop_read(BlockDriverState *bs, int64_t sector_num,
     BDRVCloopState *s = bs->opaque;
     int i;
 
-    for(i=0;i<nb_sectors;i++) {
-	uint32_t sector_offset_in_block=((sector_num+i)%s->sectors_per_block),
-	    block_num=(sector_num+i)/s->sectors_per_block;
-	if(cloop_read_block(bs, block_num) != 0)
-	    return -1;
-	memcpy(buf+i*512,s->uncompressed_block+sector_offset_in_block*512,512);
+    for (i = 0; i < nb_sectors; i++) {
+        uint32_t sector_offset_in_block =
+            ((sector_num + i) % s->sectors_per_block),
+            block_num = (sector_num + i) / s->sectors_per_block;
+        if (cloop_read_block(bs, block_num) != 0) {
+            return -1;
+        }
+        memcpy(buf + i * 512,
+            s->uncompressed_block + sector_offset_in_block * 512, 512);
     }
     return 0;
 }
@@ -160,20 +170,21 @@ static coroutine_fn int cloop_co_read(BlockDriverState *bs, int64_t sector_num,
 static void cloop_close(BlockDriverState *bs)
 {
     BDRVCloopState *s = bs->opaque;
-    if(s->n_blocks>0)
-	free(s->offsets);
-    free(s->compressed_block);
-    free(s->uncompressed_block);
+    if (s->n_blocks > 0) {
+        g_free(s->offsets);
+    }
+    g_free(s->compressed_block);
+    g_free(s->uncompressed_block);
     inflateEnd(&s->zstream);
 }
 
 static BlockDriver bdrv_cloop = {
-    .format_name	= "cloop",
-    .instance_size	= sizeof(BDRVCloopState),
-    .bdrv_probe		= cloop_probe,
-    .bdrv_open		= cloop_open,
-    .bdrv_read          = cloop_co_read,
-    .bdrv_close		= cloop_close,
+    .format_name    = "cloop",
+    .instance_size  = sizeof(BDRVCloopState),
+    .bdrv_probe     = cloop_probe,
+    .bdrv_open      = cloop_open,
+    .bdrv_read      = cloop_co_read,
+    .bdrv_close     = cloop_close,
 };
 
 static void bdrv_cloop_init(void)
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] use g_free, instead of free
  2011-11-01  6:21 [Qemu-devel] [PATCH v2] use g_free, instead of free Dong Xu Wang
@ 2011-11-01  7:54 ` Stefan Hajnoczi
  2011-11-01  8:41 ` Andreas Färber
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2011-11-01  7:54 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: Dong Xu Wang, qemu-devel

On Tue, Nov 01, 2011 at 02:21:53PM +0800, Dong Xu Wang wrote:
> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> 
> Fix mismatching allocation and deallocation: g_free should be used to pair with g_malloc.
> Also fix coding style.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> ---
>  block/cloop.c |  119 +++++++++++++++++++++++++++++++--------------------------
>  1 files changed, 65 insertions(+), 54 deletions(-)

Kevin: Please consider this for the block tree.

Stefan

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH v2] use g_free, instead of free
  2011-11-01  6:21 [Qemu-devel] [PATCH v2] use g_free, instead of free Dong Xu Wang
  2011-11-01  7:54 ` Stefan Hajnoczi
@ 2011-11-01  8:41 ` Andreas Färber
  1 sibling, 0 replies; 3+ messages in thread
From: Andreas Färber @ 2011-11-01  8:41 UTC (permalink / raw)
  To: Dong Xu Wang; +Cc: qemu-trivial, raywang, qemu-devel, stefanha

Am 01.11.2011 07:21, schrieb Dong Xu Wang:
> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> 
> Fix mismatching allocation and deallocation: g_free should be used to pair with g_malloc.
> Also fix coding style.
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>

I took the time to go through the changes. Me, I would've preferred this
to be two patches (one cleanup, one fix), since the style changes make
up the majority of this patch... Two style changes are missing for
perfection, cf. inline.

Changelog is missing. Did just the description change since v1? In that
case Ray Wang's Reviewed-by is missing. Otherwise please describe!

Trusting Ray that g_free() was right in the first place,

Reviewed-by: Andreas Färber <afaerber@suse.de>

> ---
>  block/cloop.c |  119 +++++++++++++++++++++++++++++++--------------------------
>  1 files changed, 65 insertions(+), 54 deletions(-)
> 
> diff --git a/block/cloop.c b/block/cloop.c
> index 775f8a9..1884b8c 100644
> --- a/block/cloop.c
> +++ b/block/cloop.c

> @@ -74,26 +76,28 @@ static int cloop_open(BlockDriverState *bs, int flags)
>      s->offsets = g_malloc(offsets_size);
>      if (bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size) <
>              offsets_size) {
> -	goto cloop_close;
> +        goto cloop_close;
>      }
>      for(i=0;i<s->n_blocks;i++) {
> -	s->offsets[i]=be64_to_cpu(s->offsets[i]);
> -	if(i>0) {
> -	    uint32_t size=s->offsets[i]-s->offsets[i-1];
> -	    if(size>max_compressed_block_size)
> -		max_compressed_block_size=size;
> -	}
> +        s->offsets[i] = be64_to_cpu(s->offsets[i]);
> +        if (i > 0) {
> +            uint32_t size = s->offsets[i] - s->offsets[i-1];

i - 1 theoretically

> +            if (size > max_compressed_block_size) {
> +                max_compressed_block_size = size;
> +            }
> +        }
>      }
>  
>      /* initialize zlib engine */
> -    s->compressed_block = g_malloc(max_compressed_block_size+1);
> +    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;
> -    s->current_block=s->n_blocks;
> +    if (inflateInit(&s->zstream) != Z_OK) {
> +        goto cloop_close;
> +    }
> +    s->current_block = s->n_blocks;
>  
>      s->sectors_per_block = s->block_size/512;
> -    bs->total_sectors = s->n_blocks*s->sectors_per_block;
> +    bs->total_sectors = s->n_blocks * s->sectors_per_block;
>      qemu_co_mutex_init(&s->lock);
>      return 0;
>  
> @@ -105,27 +109,30 @@ static inline int cloop_read_block(BlockDriverState *bs, int block_num)
>  {
>      BDRVCloopState *s = bs->opaque;
>  
> -    if(s->current_block != block_num) {
> -	int ret;
> -        uint32_t bytes = s->offsets[block_num+1]-s->offsets[block_num];
> +    if (s->current_block != block_num) {
> +        int ret;
> +        uint32_t bytes = s->offsets[block_num + 1]-s->offsets[block_num];

] - s

>  
>          ret = bdrv_pread(bs->file, s->offsets[block_num], s->compressed_block,
>                           bytes);
> -        if (ret != bytes)
> +        if (ret != bytes) {
>              return -1;
> +        }
> +
> +        s->zstream.next_in = s->compressed_block;
> +        s->zstream.avail_in = bytes;
> +        s->zstream.next_out = s->uncompressed_block;
> +        s->zstream.avail_out = s->block_size;
> +        ret = inflateReset(&s->zstream);
> +        if (ret != Z_OK) {
> +            return -1;
> +        }
> +        ret = inflate(&s->zstream, Z_FINISH);
> +        if (ret != Z_STREAM_END || s->zstream.total_out != s->block_size) {
> +            return -1;
> +        }
>  
> -	s->zstream.next_in = s->compressed_block;
> -	s->zstream.avail_in = bytes;
> -	s->zstream.next_out = s->uncompressed_block;
> -	s->zstream.avail_out = s->block_size;
> -	ret = inflateReset(&s->zstream);
> -	if(ret != Z_OK)
> -	    return -1;
> -	ret = inflate(&s->zstream, Z_FINISH);
> -	if(ret != Z_STREAM_END || s->zstream.total_out != s->block_size)
> -	    return -1;
> -
> -	s->current_block = block_num;
> +        s->current_block = block_num;
>      }
>      return 0;
>  }

> @@ -160,20 +170,21 @@ static coroutine_fn int cloop_co_read(BlockDriverState *bs, int64_t sector_num,
>  static void cloop_close(BlockDriverState *bs)
>  {
>      BDRVCloopState *s = bs->opaque;
> -    if(s->n_blocks>0)
> -	free(s->offsets);
> -    free(s->compressed_block);
> -    free(s->uncompressed_block);
> +    if (s->n_blocks > 0) {
> +        g_free(s->offsets);
> +    }
> +    g_free(s->compressed_block);
> +    g_free(s->uncompressed_block);

Here are the 3 functional changes!

>      inflateEnd(&s->zstream);
>  }
>  
>  static BlockDriver bdrv_cloop = {
> -    .format_name	= "cloop",
> -    .instance_size	= sizeof(BDRVCloopState),
> -    .bdrv_probe		= cloop_probe,
> -    .bdrv_open		= cloop_open,
> -    .bdrv_read          = cloop_co_read,
> -    .bdrv_close		= cloop_close,
> +    .format_name    = "cloop",
> +    .instance_size  = sizeof(BDRVCloopState),
> +    .bdrv_probe     = cloop_probe,
> +    .bdrv_open      = cloop_open,
> +    .bdrv_read      = cloop_co_read,
> +    .bdrv_close     = cloop_close,
>  };
>  
>  static void bdrv_cloop_init(void)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-11-01  9:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-01  6:21 [Qemu-devel] [PATCH v2] use g_free, instead of free Dong Xu Wang
2011-11-01  7:54 ` Stefan Hajnoczi
2011-11-01  8:41 ` Andreas Färber

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).