[v2,17/23] vmdk: Clean up control flow in vmdk_parse_extents() a bit

Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/vmdk.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

On 12/17/2015 09:49 AM, Markus Armbruster wrote:
> Cc: Fam Zheng <famz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vmdk.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

Could have mentioned what the change was: factoring out a common
next_line() helper to let you drop an end-of-loop label.

Reviewed-by: Eric Blake <eblake@redhat.com>

On Thu, 12/17 17:49, Markus Armbruster wrote:
> Cc: Fam Zheng <famz@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vmdk.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index b4a224e..08fa3f3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -760,6 +760,17 @@ static int vmdk_open_sparse(BlockDriverState *bs, BdrvChild *file, int flags,
>      }
>  }
>  
> +static const char *next_line(const char *s)
> +{
> +    while (*s) {
> +        if (*s == '\n') {
> +            return s + 1;
> +        }
> +        s++;
> +    }
> +    return s;
> +}
> +
>  static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>                                const char *desc_file_path, QDict *options,
>                                Error **errp)
> @@ -769,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 = desc;
> +    const char *p;
>      int64_t sectors = 0;
>      int64_t flat_offset;
>      char *extent_path;
> @@ -779,7 +790,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>      char extent_opt_prefix[32];
>      Error *local_err = NULL;
>  
> -    while (*p) {
> +    for (p = desc; *p; p = next_line(p)) {
>          /* parse extent line in one of below formats:
>           *
>           * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
> @@ -791,7 +802,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>          matches = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
>                           access, &sectors, type, fname, &flat_offset);
>          if (matches < 4 || strcmp(access, "RW")) {
> -            goto next_line;
> +            continue;
>          } else if (!strcmp(type, "FLAT")) {
>              if (matches != 5 || flat_offset < 0) {
>                  error_setg(errp, "Invalid extent lines: \n%s", p);
> @@ -813,7 +824,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>              (strcmp(type, "FLAT") && strcmp(type, "SPARSE") &&
>               strcmp(type, "VMFS") && strcmp(type, "VMFSSPARSE")) ||
>              (strcmp(access, "RW"))) {
> -            goto next_line;
> +            continue;
>          }
>  
>          if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
> @@ -870,15 +881,6 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>              return -ENOTSUP;
>          }
>          extent->type = g_strdup(type);
> -next_line:
> -        /* move to next line */
> -        while (*p) {
> -            if (*p == '\n') {
> -                p++;
> -                break;
> -            }
> -            p++;
> -        }
>      }
>      return 0;
>  }
> -- 
> 2.4.3
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Eric Blake <eblake@redhat.com> writes:

> On 12/17/2015 09:49 AM, Markus Armbruster wrote:
>> Cc: Fam Zheng <famz@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  block/vmdk.c | 28 +++++++++++++++-------------
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> Could have mentioned what the change was: factoring out a common
> next_line() helper to let you drop an end-of-loop label.

Adding:

    Factor out loop stepping to turn a while-loop with goto into a
    for-loop with continue.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!