Re: [Qemu-devel] [PATCH v2] allow reading variable size vmdk descriptor files

[Stefan Hajnoczi]

On Wed, Jun 12, 2013 at 02:06:30PM +0300, Evgeny Budilovsky wrote:
> @@ -719,27 +719,40 @@ static int vmdk_open_desc_file(BlockDriverState *bs, 
> int flags,
>                                 int64_t desc_offset)
>  {
>      int ret;
> -    char buf[2048];
> +    char *buf = NULL;
>      char ct[128];
>      BDRVVmdkState *s = bs->opaque;
> +    int64_t size;
> 
> -    ret = bdrv_pread(bs->file, desc_offset, buf, sizeof(buf));
> +    size = bdrv_getlength(bs->file);
> +    if (size < 0) {
> +        return -EINVAL;
> +    }
> +
> +    size = MIN(size, 1 << 20);  /* avoid unbounded allocation */

I think this is okay, initially I was worried that this function might
be called to probe files which we have not identified as descriptor
files yet (they could be big!).

But looking at the callers, it should be reasonable to read up to 1 MB.
(Would have been bad to read 1 MB just to check if this parses as a
descriptor file.)

> +    buf = g_malloc0(size + 1);
> +
> +    ret = bdrv_pread(bs->file, desc_offset, buf, size);
>      if (ret < 0) {
> -        return ret;
> +        goto exit;
>      }
> -    buf[2047] = '\0';

Thanks for pointing out the g_malloc0(), I missed it.

Reviewed-by: Stefan Hajnoczi <address@hidden>