From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43585)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1a9jHD-00069f-9t
	for qemu-devel@nongnu.org; Thu, 17 Dec 2015 19:52:08 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1a9jHA-0000LV-12
	for qemu-devel@nongnu.org; Thu, 17 Dec 2015 19:52:07 -0500
Received: from mx1.redhat.com ([209.132.183.28]:49961)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1a9jH9-0000LO-Q0
	for qemu-devel@nongnu.org; Thu, 17 Dec 2015 19:52:03 -0500
Received: from int-mx09.intmail.prod.int.phx2.redhat.com
	(int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (Postfix) with ESMTPS id E72FFAAC
	for <qemu-devel@nongnu.org>; Fri, 18 Dec 2015 00:52:02 +0000 (UTC)
Date: Fri, 18 Dec 2015 08:52:00 +0800
From: Fam Zheng <famz@redhat.com>
Message-ID: <20151218005200.GC25529@ad.usersys.redhat.com>
References: <1450371004-26866-1-git-send-email-armbru@redhat.com>
	<1450371004-26866-19-git-send-email-armbru@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1450371004-26866-19-git-send-email-armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 18/23] vmdk: Clean up "Invalid extent
 lines" error message
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org

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:
>     <first line that doesn't parse>
>     <remaining text that may or may not parse, if any>
> 
> Error messages should not have newlines embedded.  Since the remaining
> text is not helpful, we can simply report:
> 
>     Invalid extent line: <first line that doesn't parse>
> 
> Cc: Fam Zheng <famz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  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
>