Re: [PATCH 1/9] monitor: uninline add_init_drive

[Maxim Levitsky <mlevitsk@redhat.com>]

On Wed, 2019-11-27 at 08:13 +0100, Markus Armbruster wrote:
> Maxim Levitsky <mlevitsk@redhat.com> writes:
> 
> > This is only used by hmp_drive_add.
> > The code is just a bit shorter this way.
> > 
> > No functional changes
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >  device-hotplug.c | 33 +++++++++++++--------------------
> >  1 file changed, 13 insertions(+), 20 deletions(-)
> > 
> > diff --git a/device-hotplug.c b/device-hotplug.c
> > index f01d53774b..5ce73f0cff 100644
> > --- a/device-hotplug.c
> > +++ b/device-hotplug.c
> > @@ -34,42 +34,35 @@
> >  #include "monitor/monitor.h"
> >  #include "block/block_int.h"
> >  
> > -static DriveInfo *add_init_drive(const char *optstr)
> > +
> > +void hmp_drive_add(Monitor *mon, const QDict *qdict)
> >  {
> >      Error *err = NULL;
> > -    DriveInfo *dinfo;
> > +    DriveInfo *dinfo = NULL;
> 
> Superfluous initializer.
True, fixed now.
> 
> >      QemuOpts *opts;
> >      MachineClass *mc;
> > +    const char *optstr = qdict_get_str(qdict, "opts");
> > +    bool node = qdict_get_try_bool(qdict, "node", false);
> > +
> > +    if (node) {
> > +        hmp_drive_add_node(mon, optstr);
> > +        return;
> > +    }
> >  
> >      opts = drive_def(optstr);
> >      if (!opts)
> > -        return NULL;
> > +        return;
> >  
> >      mc = MACHINE_GET_CLASS(current_machine);
> >      dinfo = drive_new(opts, mc->block_default_type, &err);
> >      if (err) {
> >          error_report_err(err);
> >          qemu_opts_del(opts);
> > -        return NULL;
> > -    }
> > -
> > -    return dinfo;
> > -}
> > -
> > -void hmp_drive_add(Monitor *mon, const QDict *qdict)
> > -{
> > -    DriveInfo *dinfo = NULL;
> > -    const char *opts = qdict_get_str(qdict, "opts");
> > -    bool node = qdict_get_try_bool(qdict, "node", false);
> > -
> > -    if (node) {
> > -        hmp_drive_add_node(mon, opts);
> > -        return;
> > +        goto err;
> >      }
> >  
> > -    dinfo = add_init_drive(opts);
> >      if (!dinfo) {
> > -        goto err;
> > +        return;
> >      }
> >  
> >      switch (dinfo->type) {
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> 

Best regards,
	Maxim Levitsky