On Mon, Jun 27, 2011 at 12:54 PM, Stefan Hajnoczi <stefa...@gmail.com> wrote: > On Mon, Jun 27, 2011 at 4:48 AM, Fam Zheng <famc...@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