On Sat, Dec 27, 2014 at 04:01:35PM +0100, Peter Wu wrote: > diff --git a/block/dmg.c b/block/dmg.c > index e455886..df274f9 100644 > --- a/block/dmg.c > +++ b/block/dmg.c > @@ -131,6 +131,39 @@ static void update_max_chunk_size(BDRVDMGState *s, > uint32_t chunk, > } > } > > +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs) > +{ > + int64_t length; > + int64_t offset = 0; > + uint8_t buffer[515]; > + int i, ret; > + > + /* bdrv_getlength returns a multiple of block size (512), rounded up. > Since > + * dmg images can have odd sizes, try to look for the "koly" magic which > + * marks the begin of the UDIF trailer (512 bytes). This magic can be > found > + * in the last 511 bytes of the second-last sector or the first 4 bytes > of > + * the last sector (search space: 515 bytes) */ > + length = bdrv_getlength(file_bs); > + if (length < 512) { > + return length < 0 ? length : -EINVAL;
dmg_open() should pass in Error *errp so a detailed error reporting can be used: if (length < 0) { error_setg_errno(errp, -length, "Failed to get file size while reading UDIF trailer"); return length; } else if (length < 512) { error_set(errp, "dmg file must be at least 512 bytes long"); return -EINVAL; } This makes it much easier to pinpoint errors (instead of just -EINVAL) and also gives the user a hint about the cause. > + } > + if (length > 511 + 512) { > + offset = length - 511 - 512; > + } > + length = length < 515 ? length : 515; > + ret = bdrv_pread(file_bs, offset, buffer, length); > + if (ret < 4) { > + return ret < 0 ? ret : -EINVAL; bdrv_pread() does not return short reads. The return value will either be length or an error. This could be just: if (ret < 0) { error_setg_errno(errp, -ret, "Failed to read last sectors in dmg file"); return ret; } (The unique error string makes it easy to track down the location where the error occurs.) > + } > + for (i = 0; i < length - 3; i++) { > + if (buffer[i] == 'k' && buffer[i+1] == 'o' && > + buffer[i+2] == 'l' && buffer[i+3] == 'y') { > + return offset + i; > + } > + } > + return -EINVAL; error_set(errp, "Not a dmg file, unable to find UDIF footer"); return -EINVAL;
pgpL9AkxcAsL2.pgp
Description: PGP signature