Skip to contentSkip to conversation

block.c: Correct the return value and the failure object when failure occurs

PATCH In review Chen Gang 2 participants 3 messages Jun 21, 2014 – Jun 23, 2014

lore
Share & tools
lore

Chen Gang

author
#0 lore raw

PATCH 1/1 block.c: Correct the return value and the failure object when failure occurs

When failure occurs, 'ret' need be set, or may return 0 to indicate success. And error_propagate() also need be called only one time within a function.

Signed-off-by Chen Gang <[email protected]>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block.c +1 -1
index 74af8d7..12ce4e4 100644
@@ -1497,7 +1497,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
     if (snapshot_flags) {
         bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
         if (local_err) {
-            error_propagate(errp, local_err);
+            ret = -1;
             goto close_and_fail;
         }
     }
Signature · 1 line
1.7.11.7
#1 lore raw
Quoted 13 lines · Chen Gang

When failure occurs, 'ret' need be set, or may return 0 to indicate success. And error_propagate() also need be called only one time within a function.

Signed-off-by Chen Gang <[email protected]>
---
 block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block.c +0 -0
index 74af8d7..12ce4e4 100644

block.c int bdrv_open(BlockDriverState **pbs, const char *filename, · line 1503

Quoted 8 lines
block.c +1 -1
@@ -1497,7 +1497,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
     if (snapshot_flags) {
         bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
         if (local_err) {
-            error_propagate(errp, local_err);
+            ret = -1;
             goto close_and_fail;
         }
     }

The return value of bdrv_open() is a negative errno. -1 does not make sense in this case.

Please add an int return value to bdrv_append_temp_snapshot() so the caller can determine the real errno. Then the remainder of the patch can be:

     if (snapshot_flags) {
-        bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
+        ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
         if (local_err) {
-            error_propagate(errp, local_err);
             goto close_and_fail;
         }
     }

Stefan

author
#2 lore raw
Quoted 10 lines · Stefan Hajnoczi

The return value of bdrv_open() is a negative errno. -1 does not make sense in this case.

Please add an int return value to bdrv_append_temp_snapshot() so the caller can determine the real errno. Then the remainder of the patch can be:

     if (snapshot_flags) {

block.c int bdrv_open(BlockDriverState **pbs, const char *filename, · line 1503

Quoted 7 lines
block.c +1 -2
-        bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
+        ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
         if (local_err) {
-            error_propagate(errp, local_err);
             goto close_and_fail;
         }
     }

That's fine to me, I shall send patch v2 for it.

Thanks.

Signature · 3 lines
Chen Gang

Open share and attitude like air water and life which God blessed