Re: [Qemu-devel] [PATCH] qemu-img: don't shadow opts variable in img_dd()

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]

On 2017-06-19 17:00, Stefan Hajnoczi wrote:
> It's confusing when two different variables have the same name in one
> function.
> 
> Cc: Reda Sallahi <fullmanet@gmail.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  qemu-img.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 0ad698d..c285c2f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -4249,15 +4249,12 @@ static int img_dd(int argc, char **argv)
>          case 'U':
>              force_share = true;
>              break;
> -        case OPTION_OBJECT: {
> -            QemuOpts *opts;
> -            opts = qemu_opts_parse_noisily(&qemu_object_opts,
> -                                           optarg, true);
> -            if (!opts) {
> +        case OPTION_OBJECT:
> +            if (!qemu_opts_parse_noisily(&qemu_object_opts, optarg, true)) {
>                  ret = -1;
>                  goto out;
>              }
> -        }   break;
> +            break;
>          case OPTION_IMAGE_OPTS:
>              image_opts = true;
>              break;

Hm, I basically reverted such a style in commit
3258b91141090b05edcaab8f1d1dd355ca91b49a. I find it confusing to use the
same variable for two different things.

I agree that shadowing is bad, though. How about just renaming this
variable here?

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]