On Mon, Jun 19, 2017 at 05:18:18PM +0200, Max Reitz wrote:
> 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 don't follow how the commit you posted is related to this patch.  Did
you read the patch too quickly and think it uses the outer opts
variable?

This patch doesn't use a variable at all - there is no need for one.

Stefan