| Message ID | 53A59DD1.2020706@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Jun 21, 2014 at 10:59:29PM +0800, Chen Gang wrote: > 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 <gang.chen.5i5j@gmail.com> > --- > block.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index 74af8d7..12ce4e4 100644 > --- a/block.c > +++ b/block.c > @@ -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
On 06/23/2014 03:38 PM, Stefan Hajnoczi wrote: > > 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; > } > } > That's fine to me, I shall send patch v2 for it. Thanks.
diff --git a/block.c b/block.c index 74af8d7..12ce4e4 100644 --- a/block.c +++ b/block.c @@ -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; } }
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 <gang.chen.5i5j@gmail.com> --- block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)