diff mbox

[v2,1/6] block: vmdk - make ret variable usage clear

Message ID c34dfe0e60b35fa24730ec1cd1ddf2cc064a083a.1421768887.git.jcody@redhat.com
State New
Headers show

Commit Message

Jeff Cody Jan. 20, 2015, 5:31 p.m. UTC 
Keep the variable 'ret' something that is returned by the function it is
defined in.  For the return value of 'sscanf', use a more meaningful
variable name.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/vmdk.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

John Snow Jan. 20, 2015, 6:37 p.m. UTC | #1 
On 01/20/2015 12:31 PM, Jeff Cody wrote:
> Keep the variable 'ret' something that is returned by the function it is
> defined in.  For the return value of 'sscanf', use a more meaningful
> variable name.
>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>   block/vmdk.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 52cb888..dc6459c 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -785,6 +785,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>                                 const char *desc_file_path, Error **errp)
>   {
>       int ret;
> +    int matches;
>       char access[11];
>       char type[11];
>       char fname[512];
> @@ -796,6 +797,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>       BDRVVmdkState *s = bs->opaque;
>       VmdkExtent *extent;
>
> +

Stray newline.

>       while (*p) {
>           /* parse extent line in one of below formats:
>            *
> @@ -805,23 +807,23 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>            * RW [size in sectors] VMFSSPARSE "file-name.vmdk"
>            */
>           flat_offset = -1;
> -        ret = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
> -                access, &sectors, type, fname, &flat_offset);
> -        if (ret < 4 || strcmp(access, "RW")) {
> +        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;
>           } else if (!strcmp(type, "FLAT")) {
> -            if (ret != 5 || flat_offset < 0) {
> +            if (matches != 5 || flat_offset < 0) {
>                   error_setg(errp, "Invalid extent lines: \n%s", p);
>                   return -EINVAL;
>               }
>           } else if (!strcmp(type, "VMFS")) {
> -            if (ret == 4) {
> +            if (matches == 4) {
>                   flat_offset = 0;
>               } else {
>                   error_setg(errp, "Invalid extent lines:\n%s", p);
>                   return -EINVAL;
>               }
> -        } else if (ret != 4) {
> +        } else if (matches != 4) {
>               error_setg(errp, "Invalid extent lines:\n%s", p);
>               return -EINVAL;
>           }
>

Reviewed-by: John Snow <jsnow@redhat.com>
Stefan Hajnoczi Jan. 22, 2015, 10:56 a.m. UTC | #2 
On Tue, Jan 20, 2015 at 12:31:28PM -0500, Jeff Cody wrote:
> Keep the variable 'ret' something that is returned by the function it is
> defined in.  For the return value of 'sscanf', use a more meaningful
> variable name.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vmdk.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox

Patch

diff --git a/block/vmdk.c b/block/vmdk.c
index 52cb888..dc6459c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -785,6 +785,7 @@  static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
                               const char *desc_file_path, Error **errp)
 {
     int ret;
+    int matches;
     char access[11];
     char type[11];
     char fname[512];
@@ -796,6 +797,7 @@  static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     BDRVVmdkState *s = bs->opaque;
     VmdkExtent *extent;
 
+
     while (*p) {
         /* parse extent line in one of below formats:
          *
@@ -805,23 +807,23 @@  static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
          * RW [size in sectors] VMFSSPARSE "file-name.vmdk"
          */
         flat_offset = -1;
-        ret = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\n\r\"]\" %" SCNd64,
-                access, &sectors, type, fname, &flat_offset);
-        if (ret < 4 || strcmp(access, "RW")) {
+        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;
         } else if (!strcmp(type, "FLAT")) {
-            if (ret != 5 || flat_offset < 0) {
+            if (matches != 5 || flat_offset < 0) {
                 error_setg(errp, "Invalid extent lines: \n%s", p);
                 return -EINVAL;
             }
         } else if (!strcmp(type, "VMFS")) {
-            if (ret == 4) {
+            if (matches == 4) {
                 flat_offset = 0;
             } else {
                 error_setg(errp, "Invalid extent lines:\n%s", p);
                 return -EINVAL;
             }
-        } else if (ret != 4) {
+        } else if (matches != 4) {
             error_setg(errp, "Invalid extent lines:\n%s", p);
             return -EINVAL;
         }