On Fri, Feb 1, 2013 at 2:28 PM, Stefan Hajnoczi <stefa...@redhat.com> wrote: > From: Kevin Wolf <kw...@redhat.com> > > Return -errno instead of -1 on errors and add error checks in some > places that didn't have one. Passing things by reference requires more > correct typing, replaced a few off_ts therefore - with a 32-bit off_t > this is even a fix for truncation bugs. > > While touching the code, fix even some more memory leaks than in the > other drivers... > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > --- > block/dmg.c | 135 > +++++++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 97 insertions(+), 38 deletions(-) > > diff --git a/block/dmg.c b/block/dmg.c > index ac397dc..53be25d 100644 > --- a/block/dmg.c > +++ b/block/dmg.c > @@ -57,29 +57,42 @@ static int dmg_probe(const uint8_t *buf, int buf_size, > const char *filename) > return 0; > } > > -static off_t read_off(BlockDriverState *bs, int64_t offset) > +static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t > *result) > { > - uint64_t buffer; > - if (bdrv_pread(bs->file, offset, &buffer, 8) < 8) > - return 0; > - return be64_to_cpu(buffer); > + uint64_t buffer; > + int ret; > + > + ret = bdrv_pread(bs->file, offset, &buffer, 8); > + if (ret < 0) { > + return ret; > + } > + > + *result = be64_to_cpu(buffer); > + return 0; > } > > -static off_t read_uint32(BlockDriverState *bs, int64_t offset) > +static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t > *result) > { > - uint32_t buffer; > - if (bdrv_pread(bs->file, offset, &buffer, 4) < 4) > - return 0; > - return be32_to_cpu(buffer); > + uint32_t buffer; > + int ret; > + > + ret = bdrv_pread(bs->file, offset, &buffer, 4); > + if (ret < 0) { > + return ret; > + } > + > + *result = be32_to_cpu(buffer); > + return 0; > } > > static int dmg_open(BlockDriverState *bs, int flags) > { > BDRVDMGState *s = bs->opaque; > - off_t info_begin,info_end,last_in_offset,last_out_offset; > - uint32_t count; > + uint64_t info_begin,info_end,last_in_offset,last_out_offset; > + uint32_t count, tmp; > uint32_t max_compressed_size=1,max_sectors_per_chunk=1,i; > int64_t offset; > + int ret; > > bs->read_only = 1; > s->n_chunks = 0; > @@ -88,21 +101,32 @@ static int dmg_open(BlockDriverState *bs, int flags) > /* read offset of info blocks */ > offset = bdrv_getlength(bs->file); > if (offset < 0) { > + ret = offset; > goto fail; > } > offset -= 0x1d8; > > - info_begin = read_off(bs, offset); > - if (info_begin == 0) { > - goto fail; > + ret = read_uint64(bs, offset, &info_begin); > + if (ret < 0) { > + goto fail; > + } else if (info_begin == 0) { > + ret = -EINVAL; > + goto fail; > } > > - if (read_uint32(bs, info_begin) != 0x100) { > + ret = read_uint32(bs, info_begin, &tmp); > + if (ret < 0) { > + goto fail; > + } else if (tmp != 0x100) { > + ret = -EINVAL; > goto fail; > } > > - count = read_uint32(bs, info_begin + 4); > - if (count == 0) { > + ret = read_uint32(bs, info_begin + 4, &count); > + if (ret < 0) { > + goto fail; > + } else if (count == 0) { > + ret = -EINVAL; > goto fail; > } > info_end = info_begin + count; > @@ -114,12 +138,20 @@ static int dmg_open(BlockDriverState *bs, int flags) > while (offset < info_end) { > uint32_t type; > > - count = read_uint32(bs, offset); > - if(count==0) > - goto fail; > + ret = read_uint32(bs, offset, &count); > + if (ret < 0) { > + goto fail; > + } else if (count == 0) { > + ret = -EINVAL; > + goto fail; > + } > offset += 4; > > - type = read_uint32(bs, offset); > + ret = read_uint32(bs, offset, &type); > + if (ret < 0) { > + goto fail; > + } > + > if (type == 0x6d697368 && count >= 244) { > int new_size, chunk_count; > > @@ -134,8 +166,11 @@ static int dmg_open(BlockDriverState *bs, int flags) > s->sectors = g_realloc(s->sectors, new_size); > s->sectorcounts = g_realloc(s->sectorcounts, new_size); > > - for(i=s->n_chunks;i<s->n_chunks+chunk_count;i++) { > - s->types[i] = read_uint32(bs, offset); > + for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) { > + ret = read_uint32(bs, offset, &s->types[i]); > + if (ret < 0) { > + goto fail; > + } > offset += 4; > if(s->types[i]!=0x80000005 && s->types[i]!=1 && > s->types[i]!=2) { > if(s->types[i]==0xffffffff) { > @@ -149,17 +184,31 @@ static int dmg_open(BlockDriverState *bs, int flags) > } > offset += 4; > > - s->sectors[i] = last_out_offset+read_off(bs, offset); > - offset += 8; > - > - s->sectorcounts[i] = read_off(bs, offset); > - offset += 8; > - > - s->offsets[i] = last_in_offset+read_off(bs, offset); > - offset += 8; > - > - s->lengths[i] = read_off(bs, offset); > - offset += 8; > + ret = read_uint64(bs, offset, &s->sectors[i]); > + if (ret < 0) { > + goto fail; > + } > + s->sectors[i] += last_out_offset; > + offset += 8; > + > + ret = read_uint64(bs, offset, &s->sectorcounts[i]); > + if (ret < 0) { > + goto fail; > + } > + offset += 8; > + > + ret = read_uint64(bs, offset, &s->offsets[i]); > + if (ret < 0) { > + goto fail; > + } > + s->offsets[i] += last_in_offset; > + offset += 8; > + > + ret = read_uint64(bs, offset, &s->lengths[i]); > + if (ret < 0) { > + goto fail; > + } > + offset += 8; > > if(s->lengths[i]>max_compressed_size) > max_compressed_size = s->lengths[i]; > @@ -173,15 +222,25 @@ static int dmg_open(BlockDriverState *bs, int flags) > /* initialize zlib engine */ > s->compressed_chunk = g_malloc(max_compressed_size+1); > s->uncompressed_chunk = g_malloc(512*max_sectors_per_chunk); > - if(inflateInit(&s->zstream) != Z_OK) > - goto fail; > + if(inflateInit(&s->zstream) != Z_OK) {
Please add a space between 'if' and '('. > + ret = -EINVAL; > + goto fail; > + } > > s->current_chunk = s->n_chunks; > > qemu_co_mutex_init(&s->lock); > return 0; > + > fail: > - return -1; > + g_free(s->types); > + g_free(s->offsets); > + g_free(s->lengths); > + g_free(s->sectors); > + g_free(s->sectorcounts); > + g_free(s->compressed_chunk); > + g_free(s->uncompressed_chunk); > + return ret; > } > > static inline int is_sector_in_chunk(BDRVDMGState* s, > -- > 1.8.1 > >