| Message ID | 1419692504-29373-3-git-send-email-peter@lekensteyn.nl |
|---|---|
| State | New |
| Headers | show |
On 12/27/2014 10:01 AM, Peter Wu wrote: > Extract the mish block decoder such that this can be used for other > formats in the future. A new DmgHeaderState struct is introduced to > share state while decoding. > > The code is kept unchanged as much as possible, a "fail" label is added > for example where a simple return would probably do. > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > --- > block/dmg.c | 216 +++++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 125 insertions(+), 91 deletions(-) > > diff --git a/block/dmg.c b/block/dmg.c > index df274f9..6dc6dbb 100644 > --- a/block/dmg.c > +++ b/block/dmg.c > @@ -164,19 +164,137 @@ static int64_t dmg_find_koly_offset(BlockDriverState *file_bs) > return -EINVAL; > } > > +/* used when building the sector table */ > +typedef struct DmgHeaderState { > + /* used internally by dmg_read_mish_block to remember offsets of blocks > + * across calls */ > + uint64_t last_in_offset; > + uint64_t last_out_offset; > + /* exported for dmg_open */ > + uint32_t max_compressed_size; > + uint32_t max_sectors_per_chunk; > +} DmgHeaderState; > + > +static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds, > + int64_t offset, uint32_t count) > +{ > + BDRVDMGState *s = bs->opaque; > + uint32_t type, i; > + int ret; > + size_t new_size; > + uint32_t chunk_count; > + > + ret = read_uint32(bs, offset, &type); > + if (ret < 0) { > + goto fail; > + } > + > + /* skip data that is not a valid MISH block (invalid magic or too small) */ > + if (type != 0x6d697368 || count < 244) { > + /* assume success for now */ > + return 0; > + } > + > + offset += 4; > + offset += 200; > + > + chunk_count = (count - 204) / 40; > + new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count); > + s->types = g_realloc(s->types, new_size / 2); > + s->offsets = g_realloc(s->offsets, new_size); > + s->lengths = g_realloc(s->lengths, new_size); > + 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++) { > + 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 && i > 0) { > + ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1]; > + ds->last_out_offset = s->sectors[i - 1] + > + s->sectorcounts[i - 1]; > + } > + chunk_count--; > + i--; > + offset += 36; > + continue; > + } > + offset += 4; > + > + ret = read_uint64(bs, offset, &s->sectors[i]); > + if (ret < 0) { > + goto fail; > + } > + s->sectors[i] += ds->last_out_offset; > + offset += 8; > + > + ret = read_uint64(bs, offset, &s->sectorcounts[i]); > + if (ret < 0) { > + goto fail; > + } > + offset += 8; > + > + if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { > + error_report("sector count %" PRIu64 " for chunk %" PRIu32 > + " is larger than max (%u)", > + s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX); > + ret = -EINVAL; > + goto fail; > + } > + > + ret = read_uint64(bs, offset, &s->offsets[i]); > + if (ret < 0) { > + goto fail; > + } > + s->offsets[i] += ds->last_in_offset; > + offset += 8; > + > + ret = read_uint64(bs, offset, &s->lengths[i]); > + if (ret < 0) { > + goto fail; > + } > + offset += 8; > + > + if (s->lengths[i] > DMG_LENGTHS_MAX) { > + error_report("length %" PRIu64 " for chunk %" PRIu32 > + " is larger than max (%u)", > + s->lengths[i], i, DMG_LENGTHS_MAX); > + ret = -EINVAL; > + goto fail; > + } > + > + update_max_chunk_size(s, i, &ds->max_compressed_size, > + &ds->max_sectors_per_chunk); > + } > + s->n_chunks += chunk_count; > + return 0; > + > +fail: > + return ret; > +} > + > static int dmg_open(BlockDriverState *bs, QDict *options, int flags, > Error **errp) > { > BDRVDMGState *s = bs->opaque; > - uint64_t info_begin, info_end, last_in_offset, last_out_offset; > + DmgHeaderState ds; > + uint64_t info_begin, info_end; > 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; > s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL; > + ds.last_in_offset = 0; > + ds.last_out_offset = 0; > + ds.max_compressed_size = 1; > + ds.max_sectors_per_chunk = 1; It might be nice to have a tiny initializer macro/function so that future users of functions that use this structure don't need to know the magic numbers. > /* locate the UDIF trailer */ > offset = dmg_find_koly_offset(bs->file); > @@ -210,13 +328,11 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, > } > info_end = info_begin + count; > > + /* begin of mish block */ > offset = info_begin + 0x100; > > /* read offsets */ > - last_in_offset = last_out_offset = 0; > while (offset < info_end) { > - uint32_t type; > - > ret = read_uint32(bs, offset, &count); > if (ret < 0) { > goto fail; > @@ -226,100 +342,18 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, > } > offset += 4; > > - ret = read_uint32(bs, offset, &type); > + ret = dmg_read_mish_block(bs, &ds, offset, count); > if (ret < 0) { > goto fail; > } > - > - if (type == 0x6d697368 && count >= 244) { > - size_t new_size; > - uint32_t chunk_count; > - > - offset += 4; > - offset += 200; > - > - chunk_count = (count - 204) / 40; > - new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count); > - s->types = g_realloc(s->types, new_size / 2); > - s->offsets = g_realloc(s->offsets, new_size); > - s->lengths = g_realloc(s->lengths, new_size); > - 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++) { > - 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 && i > 0) { > - last_in_offset = s->offsets[i - 1] + s->lengths[i - 1]; > - last_out_offset = s->sectors[i - 1] + > - s->sectorcounts[i - 1]; > - } > - chunk_count--; > - i--; > - offset += 36; > - continue; > - } > - offset += 4; > - > - 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; > - > - if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { > - error_report("sector count %" PRIu64 " for chunk %" PRIu32 > - " is larger than max (%u)", > - s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX); > - ret = -EINVAL; > - goto fail; > - } > - > - 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] > DMG_LENGTHS_MAX) { > - error_report("length %" PRIu64 " for chunk %" PRIu32 > - " is larger than max (%u)", > - s->lengths[i], i, DMG_LENGTHS_MAX); > - ret = -EINVAL; > - goto fail; > - } > - > - update_max_chunk_size(s, i, &max_compressed_size, > - &max_sectors_per_chunk); > - } > - s->n_chunks += chunk_count; > - } > + offset += count; A little justification here might be nice: I assume the "count" always includes the full byte count of the mish block, and the old code just incremented the offset up to this amount. This is the only functional difference in this refactor that I can see. > } > > /* initialize zlib engine */ > s->compressed_chunk = qemu_try_blockalign(bs->file, > - max_compressed_size + 1); > + ds.max_compressed_size + 1); > s->uncompressed_chunk = qemu_try_blockalign(bs->file, > - 512 * max_sectors_per_chunk); > + 512 * ds.max_sectors_per_chunk); > if (s->compressed_chunk == NULL || s->uncompressed_chunk == NULL) { > ret = -ENOMEM; > goto fail; > Thank you for splitting this monolithic function up into nicer pieces. With or without addressing my comments above: Reviewed-by: John Snow <jsnow@redhat.com>
On Friday 02 January 2015 18:59:38 John Snow wrote: > > On 12/27/2014 10:01 AM, Peter Wu wrote: > > Extract the mish block decoder such that this can be used for other > > formats in the future. A new DmgHeaderState struct is introduced to > > share state while decoding. > > > > The code is kept unchanged as much as possible, a "fail" label is added > > for example where a simple return would probably do. > > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > > --- > > block/dmg.c | 216 +++++++++++++++++++++++++++++++++++------------------------- > > 1 file changed, 125 insertions(+), 91 deletions(-) > > > > diff --git a/block/dmg.c b/block/dmg.c > > index df274f9..6dc6dbb 100644 > > --- a/block/dmg.c > > +++ b/block/dmg.c > > @@ -164,19 +164,137 @@ static int64_t dmg_find_koly_offset(BlockDriverState *file_bs) > > return -EINVAL; > > } > > > > +/* used when building the sector table */ > > +typedef struct DmgHeaderState { > > + /* used internally by dmg_read_mish_block to remember offsets of blocks > > + * across calls */ > > + uint64_t last_in_offset; > > + uint64_t last_out_offset; > > + /* exported for dmg_open */ > > + uint32_t max_compressed_size; > > + uint32_t max_sectors_per_chunk; > > +} DmgHeaderState; > > + > > +static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds, > > + int64_t offset, uint32_t count) > > +{ > > + BDRVDMGState *s = bs->opaque; > > + uint32_t type, i; > > + int ret; > > + size_t new_size; > > + uint32_t chunk_count; > > + > > + ret = read_uint32(bs, offset, &type); > > + if (ret < 0) { > > + goto fail; > > + } > > + > > + /* skip data that is not a valid MISH block (invalid magic or too small) */ > > + if (type != 0x6d697368 || count < 244) { > > + /* assume success for now */ > > + return 0; > > + } > > + > > + offset += 4; > > + offset += 200; > > + > > + chunk_count = (count - 204) / 40; > > + new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count); > > + s->types = g_realloc(s->types, new_size / 2); > > + s->offsets = g_realloc(s->offsets, new_size); > > + s->lengths = g_realloc(s->lengths, new_size); > > + 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++) { > > + 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 && i > 0) { > > + ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1]; > > + ds->last_out_offset = s->sectors[i - 1] + > > + s->sectorcounts[i - 1]; > > + } > > + chunk_count--; > > + i--; > > + offset += 36; > > + continue; > > + } > > + offset += 4; > > + > > + ret = read_uint64(bs, offset, &s->sectors[i]); > > + if (ret < 0) { > > + goto fail; > > + } > > + s->sectors[i] += ds->last_out_offset; > > + offset += 8; > > + > > + ret = read_uint64(bs, offset, &s->sectorcounts[i]); > > + if (ret < 0) { > > + goto fail; > > + } > > + offset += 8; > > + > > + if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { > > + error_report("sector count %" PRIu64 " for chunk %" PRIu32 > > + " is larger than max (%u)", > > + s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX); > > + ret = -EINVAL; > > + goto fail; > > + } > > + > > + ret = read_uint64(bs, offset, &s->offsets[i]); > > + if (ret < 0) { > > + goto fail; > > + } > > + s->offsets[i] += ds->last_in_offset; > > + offset += 8; > > + > > + ret = read_uint64(bs, offset, &s->lengths[i]); > > + if (ret < 0) { > > + goto fail; > > + } > > + offset += 8; > > + > > + if (s->lengths[i] > DMG_LENGTHS_MAX) { > > + error_report("length %" PRIu64 " for chunk %" PRIu32 > > + " is larger than max (%u)", > > + s->lengths[i], i, DMG_LENGTHS_MAX); > > + ret = -EINVAL; > > + goto fail; > > + } > > + > > + update_max_chunk_size(s, i, &ds->max_compressed_size, > > + &ds->max_sectors_per_chunk); > > + } > > + s->n_chunks += chunk_count; > > + return 0; > > + > > +fail: > > + return ret; > > +} > > + > > static int dmg_open(BlockDriverState *bs, QDict *options, int flags, > > Error **errp) > > { > > BDRVDMGState *s = bs->opaque; > > - uint64_t info_begin, info_end, last_in_offset, last_out_offset; > > + DmgHeaderState ds; > > + uint64_t info_begin, info_end; > > 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; > > s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL; > > + ds.last_in_offset = 0; > > + ds.last_out_offset = 0; > > + ds.max_compressed_size = 1; > > + ds.max_sectors_per_chunk = 1; > > It might be nice to have a tiny initializer macro/function so that > future users of functions that use this structure don't need to know the > magic numbers. This state is internal to the dmg_read_mish_block functionality for which the dmg_open function is the only caller. I consider it similar to the initialization for BDRVDMGState in the lines above so an additional indirection to initialize DmgHeaderState does not seem warranted. In the v2 patch a minor comment is added before this block. The magic number 1 existed before and seem to be in-place to avoid allocating a 0 size (for max_sectors_per_chunk). Why max_compressed_size is 1 is beyond me though, but I copied this value from the previous version. > > /* locate the UDIF trailer */ > > offset = dmg_find_koly_offset(bs->file); > > @@ -210,13 +328,11 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, > > } > > info_end = info_begin + count; > > > > + /* begin of mish block */ > > offset = info_begin + 0x100; > > > > /* read offsets */ > > - last_in_offset = last_out_offset = 0; > > while (offset < info_end) { > > - uint32_t type; > > - > > ret = read_uint32(bs, offset, &count); > > if (ret < 0) { > > goto fail; > > @@ -226,100 +342,18 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, > > } > > offset += 4; > > > > - ret = read_uint32(bs, offset, &type); > > + ret = dmg_read_mish_block(bs, &ds, offset, count); > > if (ret < 0) { > > goto fail; > > } > > - > > - if (type == 0x6d697368 && count >= 244) { > > - size_t new_size; > > - uint32_t chunk_count; > > - > > - offset += 4; > > - offset += 200; > > - > > - chunk_count = (count - 204) / 40; > > - new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count); > > - s->types = g_realloc(s->types, new_size / 2); > > - s->offsets = g_realloc(s->offsets, new_size); > > - s->lengths = g_realloc(s->lengths, new_size); > > - 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++) { > > - 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 && i > 0) { > > - last_in_offset = s->offsets[i - 1] + s->lengths[i - 1]; > > - last_out_offset = s->sectors[i - 1] + > > - s->sectorcounts[i - 1]; > > - } > > - chunk_count--; > > - i--; > > - offset += 36; > > - continue; > > - } > > - offset += 4; > > - > > - 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; > > - > > - if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { > > - error_report("sector count %" PRIu64 " for chunk %" PRIu32 > > - " is larger than max (%u)", > > - s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX); > > - ret = -EINVAL; > > - goto fail; > > - } > > - > > - 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] > DMG_LENGTHS_MAX) { > > - error_report("length %" PRIu64 " for chunk %" PRIu32 > > - " is larger than max (%u)", > > - s->lengths[i], i, DMG_LENGTHS_MAX); > > - ret = -EINVAL; > > - goto fail; > > - } > > - > > - update_max_chunk_size(s, i, &max_compressed_size, > > - &max_sectors_per_chunk); > > - } > > - s->n_chunks += chunk_count; > > - } > > + offset += count; > > A little justification here might be nice: I assume the "count" always > includes the full byte count of the mish block, and the old code just > incremented the offset up to this amount. This is the only functional > difference in this refactor that I can see. This code got me confused again and after looking at this document[1], I have added some explanatory comments for the next version. The "count" in the loop refers to the contents of a single resource. Basically we have this: Resource header - 4 bytes rsrc data offset (assume 0x100) - 4 bytes rsrc map offset (ignored) - 4 bytes rsrc data length - 4 bytes rsrc map length (ignored) The block of length "rsrc data length" at offset 0x100 consists of one or more resources, each described by: - 4 bytes resource length - "resource length" number of bytes. Its contents is a "mish" block for our purposes. Indeed, in this patch I avoided the need to copy from the mish block parser as it is not really necessary given that the resource length is known. [1]: https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151 > > } > > > > /* initialize zlib engine */ > > s->compressed_chunk = qemu_try_blockalign(bs->file, > > - max_compressed_size + 1); > > + ds.max_compressed_size + 1); > > s->uncompressed_chunk = qemu_try_blockalign(bs->file, > > - 512 * max_sectors_per_chunk); > > + 512 * ds.max_sectors_per_chunk); > > if (s->compressed_chunk == NULL || s->uncompressed_chunk == NULL) { > > ret = -ENOMEM; > > goto fail; > > > > Thank you for splitting this monolithic function up into nicer pieces. > With or without addressing my comments above: > > Reviewed-by: John Snow <jsnow@redhat.com> Since I have only added some comments and renamed a variable, I'll carry this R-b to the v2 patch.
On Sat, Dec 27, 2014 at 04:01:36PM +0100, Peter Wu wrote: > Extract the mish block decoder such that this can be used for other > formats in the future. A new DmgHeaderState struct is introduced to > share state while decoding. > > The code is kept unchanged as much as possible, a "fail" label is added > for example where a simple return would probably do. > > Signed-off-by: Peter Wu <peter@lekensteyn.nl> > --- > block/dmg.c | 216 +++++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 125 insertions(+), 91 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/block/dmg.c b/block/dmg.c index df274f9..6dc6dbb 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -164,19 +164,137 @@ static int64_t dmg_find_koly_offset(BlockDriverState *file_bs) return -EINVAL; } +/* used when building the sector table */ +typedef struct DmgHeaderState { + /* used internally by dmg_read_mish_block to remember offsets of blocks + * across calls */ + uint64_t last_in_offset; + uint64_t last_out_offset; + /* exported for dmg_open */ + uint32_t max_compressed_size; + uint32_t max_sectors_per_chunk; +} DmgHeaderState; + +static int dmg_read_mish_block(BlockDriverState *bs, DmgHeaderState *ds, + int64_t offset, uint32_t count) +{ + BDRVDMGState *s = bs->opaque; + uint32_t type, i; + int ret; + size_t new_size; + uint32_t chunk_count; + + ret = read_uint32(bs, offset, &type); + if (ret < 0) { + goto fail; + } + + /* skip data that is not a valid MISH block (invalid magic or too small) */ + if (type != 0x6d697368 || count < 244) { + /* assume success for now */ + return 0; + } + + offset += 4; + offset += 200; + + chunk_count = (count - 204) / 40; + new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count); + s->types = g_realloc(s->types, new_size / 2); + s->offsets = g_realloc(s->offsets, new_size); + s->lengths = g_realloc(s->lengths, new_size); + 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++) { + 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 && i > 0) { + ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1]; + ds->last_out_offset = s->sectors[i - 1] + + s->sectorcounts[i - 1]; + } + chunk_count--; + i--; + offset += 36; + continue; + } + offset += 4; + + ret = read_uint64(bs, offset, &s->sectors[i]); + if (ret < 0) { + goto fail; + } + s->sectors[i] += ds->last_out_offset; + offset += 8; + + ret = read_uint64(bs, offset, &s->sectorcounts[i]); + if (ret < 0) { + goto fail; + } + offset += 8; + + if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { + error_report("sector count %" PRIu64 " for chunk %" PRIu32 + " is larger than max (%u)", + s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX); + ret = -EINVAL; + goto fail; + } + + ret = read_uint64(bs, offset, &s->offsets[i]); + if (ret < 0) { + goto fail; + } + s->offsets[i] += ds->last_in_offset; + offset += 8; + + ret = read_uint64(bs, offset, &s->lengths[i]); + if (ret < 0) { + goto fail; + } + offset += 8; + + if (s->lengths[i] > DMG_LENGTHS_MAX) { + error_report("length %" PRIu64 " for chunk %" PRIu32 + " is larger than max (%u)", + s->lengths[i], i, DMG_LENGTHS_MAX); + ret = -EINVAL; + goto fail; + } + + update_max_chunk_size(s, i, &ds->max_compressed_size, + &ds->max_sectors_per_chunk); + } + s->n_chunks += chunk_count; + return 0; + +fail: + return ret; +} + static int dmg_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { BDRVDMGState *s = bs->opaque; - uint64_t info_begin, info_end, last_in_offset, last_out_offset; + DmgHeaderState ds; + uint64_t info_begin, info_end; 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; s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL; + ds.last_in_offset = 0; + ds.last_out_offset = 0; + ds.max_compressed_size = 1; + ds.max_sectors_per_chunk = 1; /* locate the UDIF trailer */ offset = dmg_find_koly_offset(bs->file); @@ -210,13 +328,11 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, } info_end = info_begin + count; + /* begin of mish block */ offset = info_begin + 0x100; /* read offsets */ - last_in_offset = last_out_offset = 0; while (offset < info_end) { - uint32_t type; - ret = read_uint32(bs, offset, &count); if (ret < 0) { goto fail; @@ -226,100 +342,18 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, } offset += 4; - ret = read_uint32(bs, offset, &type); + ret = dmg_read_mish_block(bs, &ds, offset, count); if (ret < 0) { goto fail; } - - if (type == 0x6d697368 && count >= 244) { - size_t new_size; - uint32_t chunk_count; - - offset += 4; - offset += 200; - - chunk_count = (count - 204) / 40; - new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count); - s->types = g_realloc(s->types, new_size / 2); - s->offsets = g_realloc(s->offsets, new_size); - s->lengths = g_realloc(s->lengths, new_size); - 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++) { - 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 && i > 0) { - last_in_offset = s->offsets[i - 1] + s->lengths[i - 1]; - last_out_offset = s->sectors[i - 1] + - s->sectorcounts[i - 1]; - } - chunk_count--; - i--; - offset += 36; - continue; - } - offset += 4; - - 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; - - if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { - error_report("sector count %" PRIu64 " for chunk %" PRIu32 - " is larger than max (%u)", - s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX); - ret = -EINVAL; - goto fail; - } - - 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] > DMG_LENGTHS_MAX) { - error_report("length %" PRIu64 " for chunk %" PRIu32 - " is larger than max (%u)", - s->lengths[i], i, DMG_LENGTHS_MAX); - ret = -EINVAL; - goto fail; - } - - update_max_chunk_size(s, i, &max_compressed_size, - &max_sectors_per_chunk); - } - s->n_chunks += chunk_count; - } + offset += count; } /* initialize zlib engine */ s->compressed_chunk = qemu_try_blockalign(bs->file, - max_compressed_size + 1); + ds.max_compressed_size + 1); s->uncompressed_chunk = qemu_try_blockalign(bs->file, - 512 * max_sectors_per_chunk); + 512 * ds.max_sectors_per_chunk); if (s->compressed_chunk == NULL || s->uncompressed_chunk == NULL) { ret = -ENOMEM; goto fail;
Extract the mish block decoder such that this can be used for other formats in the future. A new DmgHeaderState struct is introduced to share state while decoding. The code is kept unchanged as much as possible, a "fail" label is added for example where a simple return would probably do. Signed-off-by: Peter Wu <peter@lekensteyn.nl> --- block/dmg.c | 216 +++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 125 insertions(+), 91 deletions(-)