Re: [Qemu-devel] [PATCH v2 18/23] vmdk: Clean up "Invalid extent lines" error message
Fam Zheng writes:
> On Thu, 12/17 17:49, Markus Armbruster wrote:
>> vmdk_parse_extents() reports parse errors like this:
>>
>> error_setg(errp, "Invalid extent lines:\n%s", p);
>>
>> where p points to the beginning of the malformed line in the image
>> descriptor. This results in a multi-line error message
>>
>> Invalid extent lines:
>>
>>
>>
>> Error messages should not have newlines embedded. Since the remaining
>> text is not helpful, we can simply report:
>>
>> Invalid extent line:
>>
>> Cc: Fam Zheng
>> Signed-off-by: Markus Armbruster
>> ---
>> block/vmdk.c | 19 ---
>> tests/qemu-iotests/059.out | 4 +---
>> 2 files changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 08fa3f3..04b47e3 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -780,7 +780,7 @@ static int vmdk_parse_extents(const char *desc,
>> BlockDriverState *bs,
>> char access[11];
>> char type[11];
>> char fname[512];
>> -const char *p;
>> +const char *p, *np;
>> int64_t sectors = 0;
>> int64_t flat_offset;
>> char *extent_path;
>> @@ -805,19 +805,16 @@ static int vmdk_parse_extents(const char *desc,
>> BlockDriverState *bs,
>> continue;
>> } else if (!strcmp(type, "FLAT")) {
>> if (matches != 5 || flat_offset < 0) {
>> -error_setg(errp, "Invalid extent lines: \n%s", p);
>> -return -EINVAL;
>> +goto invalid;
>> }
>> } else if (!strcmp(type, "VMFS")) {
>> if (matches == 4) {
>> flat_offset = 0;
>> } else {
>> -error_setg(errp, "Invalid extent lines:\n%s", p);
>> -return -EINVAL;
>> +goto invalid;
>> }
>> } else if (matches != 4) {
>> -error_setg(errp, "Invalid extent lines:\n%s", p);
>> -return -EINVAL;
>> +goto invalid;
>> }
>>
>> if (sectors <= 0 ||
>> @@ -883,6 +880,14 @@ static int vmdk_parse_extents(const char *desc,
>> BlockDriverState *bs,
>> extent->type = g_strdup(type);
>> }
>> return 0;
>> +
>> +invalid:
>> +np = next_line(p);
>> +assert(np != p);
>> +if (np[-1] == '\n')
>> +np--;
>
> Braces are required.
Fixing...
>> +error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
>> +return -EINVAL;
>> }
>>
>> static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
>> diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
>> index d28df5b..9d506cb 100644
>> --- a/tests/qemu-iotests/059.out
>> +++ b/tests/qemu-iotests/059.out
>> @@ -2038,9 +2038,7 @@ Format specific information:
>> format: FLAT
>>
>> === Testing malformed VMFS extent description line ===
>> -qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent lines:
>> -RW 12582912 VMFS "dummy.IMGFMT" 1
>> -
>> +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW
>> 12582912 VMFS "dummy.IMGFMT" 1
>>
>> === Testing truncated sparse ===
>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
>> subformat=monolithicSparse
>> --
>> 2.4.3
>>
Re: [Qemu-devel] [PATCH v2 18/23] vmdk: Clean up "Invalid extent lines" error message
On Thu, 12/17 17:49, Markus Armbruster wrote:
> vmdk_parse_extents() reports parse errors like this:
>
> error_setg(errp, "Invalid extent lines:\n%s", p);
>
> where p points to the beginning of the malformed line in the image
> descriptor. This results in a multi-line error message
>
> Invalid extent lines:
>
>
>
> Error messages should not have newlines embedded. Since the remaining
> text is not helpful, we can simply report:
>
> Invalid extent line:
>
> Cc: Fam Zheng
> Signed-off-by: Markus Armbruster
> ---
> block/vmdk.c | 19 ---
> tests/qemu-iotests/059.out | 4 +---
> 2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 08fa3f3..04b47e3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -780,7 +780,7 @@ static int vmdk_parse_extents(const char *desc,
> BlockDriverState *bs,
> char access[11];
> char type[11];
> char fname[512];
> -const char *p;
> +const char *p, *np;
> int64_t sectors = 0;
> int64_t flat_offset;
> char *extent_path;
> @@ -805,19 +805,16 @@ static int vmdk_parse_extents(const char *desc,
> BlockDriverState *bs,
> continue;
> } else if (!strcmp(type, "FLAT")) {
> if (matches != 5 || flat_offset < 0) {
> -error_setg(errp, "Invalid extent lines: \n%s", p);
> -return -EINVAL;
> +goto invalid;
> }
> } else if (!strcmp(type, "VMFS")) {
> if (matches == 4) {
> flat_offset = 0;
> } else {
> -error_setg(errp, "Invalid extent lines:\n%s", p);
> -return -EINVAL;
> +goto invalid;
> }
> } else if (matches != 4) {
> -error_setg(errp, "Invalid extent lines:\n%s", p);
> -return -EINVAL;
> +goto invalid;
> }
>
> if (sectors <= 0 ||
> @@ -883,6 +880,14 @@ static int vmdk_parse_extents(const char *desc,
> BlockDriverState *bs,
> extent->type = g_strdup(type);
> }
> return 0;
> +
> +invalid:
> +np = next_line(p);
> +assert(np != p);
> +if (np[-1] == '\n')
> +np--;
Braces are required.
> +error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
> +return -EINVAL;
> }
>
> static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
> diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
> index d28df5b..9d506cb 100644
> --- a/tests/qemu-iotests/059.out
> +++ b/tests/qemu-iotests/059.out
> @@ -2038,9 +2038,7 @@ Format specific information:
> format: FLAT
>
> === Testing malformed VMFS extent description line ===
> -qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent lines:
> -RW 12582912 VMFS "dummy.IMGFMT" 1
> -
> +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW
> 12582912 VMFS "dummy.IMGFMT" 1
>
> === Testing truncated sparse ===
> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
> subformat=monolithicSparse
> --
> 2.4.3
>
Re: [Qemu-devel] [PATCH v2 18/23] vmdk: Clean up "Invalid extent lines" error message
On 12/17/2015 09:49 AM, Markus Armbruster wrote: > vmdk_parse_extents() reports parse errors like this: > > error_setg(errp, "Invalid extent lines:\n%s", p); > > where p points to the beginning of the malformed line in the image > descriptor. This results in a multi-line error message > > Invalid extent lines: > > > > Error messages should not have newlines embedded. Since the remaining > text is not helpful, we can simply report: > > Invalid extent line: > > Cc: Fam Zheng > Signed-off-by: Markus Armbruster > --- > block/vmdk.c | 19 --- > tests/qemu-iotests/059.out | 4 +--- > 2 files changed, 13 insertions(+), 10 deletions(-) > > @@ -883,6 +880,14 @@ static int vmdk_parse_extents(const char *desc, > BlockDriverState *bs, > extent->type = g_strdup(type); > } > return 0; > + > +invalid: > +np = next_line(p); > +assert(np != p); > +if (np[-1] == '\n') > +np--; > +error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p); [Side note: It would be nice if C/POSIX had a way to specify that %.*s will take a ptrdiff_t argument, instead of forcing callers to cast to int. Oh well.] Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
