Re: [PATCH 1/3] block: Simplify qmp_block_resize() error paths

[Kevin Wolf]

Am 08.12.2020 um 15:15 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.12.2020 20:23, Kevin Wolf wrote:
> > The only thing that happens after the 'out:' label is blk_unref(blk).
> > However, blk = NULL in all of the error cases, so instead of jumping to
> > 'out:', we can just return directly.
> > 
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Kevin Wolf 
> > ---
> >   blockdev.c | 7 +++
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index fe6fb5dc1d..229d2cce1b 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -2468,17 +2468,17 @@ void coroutine_fn qmp_block_resize(bool has_device, 
> > const char *device,
> >   if (size < 0) {
> >   error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 
> > size");
> > -goto out;
> > +return;
> >   }
> >   if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
> >   error_setg(errp, QERR_DEVICE_IN_USE, device);
> > -goto out;
> > +return;
> >   }
> >   blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp);
> >   if (!blk) {
> > -goto out;
> > +return;
> >   }
> >   bdrv_drained_begin(bs);
> > @@ -2487,7 +2487,6 @@ void coroutine_fn qmp_block_resize(bool has_device, 
> > const char *device,
> >   bdrv_co_leave(bs, old_ctx);
> >   bdrv_drained_end(bs);
> > -out:
> >   bdrv_co_lock(bs);
> >   blk_unref(blk);
> >   bdrv_co_unlock(bs);
> > 
> 
> Initialization of blk to NULL becomes redundant with this patch, so
> may be dropped too.

Good catch, I'll change this while applying.

Kevin

Re: [PATCH 1/3] block: Simplify qmp_block_resize() error paths

[Vladimir Sementsov-Ogievskiy]

03.12.2020 20:23, Kevin Wolf wrote:

The only thing that happens after the 'out:' label is blk_unref(blk).
However, blk = NULL in all of the error cases, so instead of jumping to
'out:', we can just return directly.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
---
  blockdev.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fe6fb5dc1d..229d2cce1b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2468,17 +2468,17 @@ void coroutine_fn qmp_block_resize(bool has_device, 
const char *device,
  
  if (size < 0) {

  error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
-goto out;
+return;
  }
  
  if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {

  error_setg(errp, QERR_DEVICE_IN_USE, device);
-goto out;
+return;
  }
  
  blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp);

  if (!blk) {
-goto out;
+return;
  }
  
  bdrv_drained_begin(bs);

@@ -2487,7 +2487,6 @@ void coroutine_fn qmp_block_resize(bool has_device, const 
char *device,
  bdrv_co_leave(bs, old_ctx);
  bdrv_drained_end(bs);
  
-out:

  bdrv_co_lock(bs);
  blk_unref(blk);
  bdrv_co_unlock(bs);



Initialization of blk to NULL becomes redundant with this patch, so may be 
dropped too. Anyway:

Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

[PATCH 1/3] block: Simplify qmp_block_resize() error paths

[Kevin Wolf]

The only thing that happens after the 'out:' label is blk_unref(blk).
However, blk = NULL in all of the error cases, so instead of jumping to
'out:', we can just return directly.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index fe6fb5dc1d..229d2cce1b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2468,17 +2468,17 @@ void coroutine_fn qmp_block_resize(bool has_device, 
const char *device,
 
 if (size < 0) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
-goto out;
+return;
 }
 
 if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
 error_setg(errp, QERR_DEVICE_IN_USE, device);
-goto out;
+return;
 }
 
 blk = blk_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp);
 if (!blk) {
-goto out;
+return;
 }
 
 bdrv_drained_begin(bs);
@@ -2487,7 +2487,6 @@ void coroutine_fn qmp_block_resize(bool has_device, const 
char *device,
 bdrv_co_leave(bs, old_ctx);
 bdrv_drained_end(bs);
 
-out:
 bdrv_co_lock(bs);
 blk_unref(blk);
 bdrv_co_unlock(bs);
-- 
2.28.0