From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8v2m-0007Fi-Ce for qemu-devel@nongnu.org; Wed, 07 Jan 2015 13:09:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8v2h-0006mM-E2 for qemu-devel@nongnu.org; Wed, 07 Jan 2015 13:09:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34155) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8v2h-0006m5-7V for qemu-devel@nongnu.org; Wed, 07 Jan 2015 13:09:15 -0500 Message-ID: <54AD7646.9050009@redhat.com> Date: Wed, 07 Jan 2015 13:09:10 -0500 From: John Snow MIME-Version: 1.0 References: <1420566495-13284-1-git-send-email-peter@lekensteyn.nl> <1420566495-13284-11-git-send-email-peter@lekensteyn.nl> In-Reply-To: <1420566495-13284-11-git-send-email-peter@lekensteyn.nl> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 10/12] block/dmg: factor out block type check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Wu , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi On 01/06/2015 12:48 PM, Peter Wu wrote: > In preparation for adding bzip2 support, split the type check into a > separate function. Make all offsets relative to the begin of a chunk > such that it is easier to recognize the position without having to > add up all offsets. Some comments are added to describe the fields. > > There is no functional change. > > Signed-off-by: Peter Wu > --- > v2: new patch, split off bzip2 patch. Besides the > dmg_is_known_block_type function, the offsets are now also made > relative. > --- > block/dmg.c | 36 +++++++++++++++++++++++------------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/block/dmg.c b/block/dmg.c > index 57922c5..b1d0930 100644 > --- a/block/dmg.c > +++ b/block/dmg.c > @@ -194,6 +194,18 @@ typedef struct DmgHeaderState { > uint32_t max_sectors_per_chunk; > } DmgHeaderState; > > +static bool dmg_is_known_block_type(uint32_t entry_type) > +{ > + switch (entry_type) { > + case 0x00000001: /* uncompressed */ > + case 0x00000002: /* zeroes */ > + case 0x80000005: /* zlib */ > + return true; > + default: > + return false; > + } > +} > + > static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds, > uint8_t *buffer, uint32_t count) > { > @@ -233,22 +245,19 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds, > > for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) { > s->types[i] = buff_read_uint32(buffer, offset); > - offset += 4; > - if (s->types[i] != 0x80000005 && s->types[i] != 1 && > - s->types[i] != 2) { > + if (!dmg_is_known_block_type(s->types[i])) { > chunk_count--; > i--; > - offset += 36; > + offset += 40; > continue; > } > - offset += 4; > > - s->sectors[i] = buff_read_uint64(buffer, offset); > + /* sector number */ > + s->sectors[i] = buff_read_uint64(buffer, offset + 8); > s->sectors[i] += out_offset; > - offset += 8; > > - s->sectorcounts[i] = buff_read_uint64(buffer, offset); > - offset += 8; > + /* sector count */ > + s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10); > > if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) { > error_report("sector count %" PRIu64 " for chunk %" PRIu32 > @@ -258,12 +267,12 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds, > goto fail; > } > > - s->offsets[i] = buff_read_uint64(buffer, offset); > + /* offset in (compressed) data fork */ > + s->offsets[i] = buff_read_uint64(buffer, offset + 0x18); > s->offsets[i] += in_offset; > - offset += 8; > > - s->lengths[i] = buff_read_uint64(buffer, offset); > - offset += 8; > + /* length in (compressed) data fork */ > + s->lengths[i] = buff_read_uint64(buffer, offset + 0x20); > > if (s->lengths[i] > DMG_LENGTHS_MAX) { > error_report("length %" PRIu64 " for chunk %" PRIu32 > @@ -275,6 +284,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds, > > update_max_chunk_size(s, i, &ds->max_compressed_size, > &ds->max_sectors_per_chunk); > + offset += 40; > } > s->n_chunks += chunk_count; > return 0; > Reviewed-by: John Snow