Re: [Qemu-devel] [PATCH v3 09/12] VMDK: open/read/write for monolithicFlat image

[Fam Zheng <famcool@gmail.com>]

On Mon, Jun 27, 2011 at 12:54 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jun 27, 2011 at 4:48 AM, Fam Zheng <famcool@gmail.com> wrote:
>> Parse vmdk decriptor file and open mono flat image.
>> @@ -598,6 +600,154 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
>>     return ret;
>>  }
>>
>> +/* find an option value out of descriptor file */
>> +static int vmdk_parse_description(const char *desc, const char *opt_name,
>> +        char *buf, int buf_size)
>> +{
>> +    char *opt_pos = strstr(desc, opt_name);
>> +    int r;
>> +    const char *end = desc + strlen(desc);
>> +
>> +    if (!opt_pos) {
>> +        return -1;
>> +    }
>> +    opt_pos += strlen(opt_name) + 2;
>> +    if (opt_pos >= end) {
>> +        return -1;
>> +    }
>> +    r = sscanf(opt_pos, "%[^\"]s", buf);
>> +    return r <= 0;
>> +}
>
> This is still unsafe.  Please see my comments on the previous version
> of this patch.
How about this:

static int vmdk_parse_description(const char *desc, const char *opt_name,
        char *buf, int buf_size)
{
    char *opt_pos, *opt_end;
    const char *end = desc + strlen(desc);

    opt_pos = strstr(desc, opt_name);
    if (!opt_pos) {
        return -1;
    }
    /* Skip "=\"" following opt_name */
    opt_pos += strlen(opt_name) + 2;
    if (opt_pos >= end) {
        return -1;
    }
    opt_end = opt_pos;
    while (opt_end < end && *opt_end != '"') {
        opt_end++;
    }
    if (opt_end == end || buf_size < opt_end - opt_pos + 1) {
        return -1;
    }
    strncpy(buf, opt_pos, opt_end - opt_pos);
    buf[opt_end - opt_pos] = '\0';
    return 0;
}
>
>> +static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>> +{
>> +    int ret;
>> +    char buf[2048];
>> +    char ct[128];
>> +    BDRVVmdkState *s = bs->opaque;
>> +
>> +    ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
>> +    ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
>
> Merge error?  Only need to bdrv_pread() once :).
>
> Stefan
>



-- 
Best regards!
Fam Zheng