| Message ID | 1320128513-16208-1-git-send-email-wdongxu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
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
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
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)