On Thu, 5 Oct 2017 14:32:17 +0200
Thomas Huth <th...@redhat.com> wrote:
> Instead of doing the clean-ups on errors multiple times, introduce
> a jump label at the end of the function that can be used by all
> error paths that need this cleanup.
>
> Suggested-by: Igor Mammedov <imamm...@redhat.com>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> qdev-monitor.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 8fd6df9..cb2b109 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -620,22 +620,21 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error
> **errp)
>
> /* set properties */
> if (qemu_opt_foreach(opts, set_property, dev, &err)) {
> - error_propagate(errp, err);
> - object_unparent(OBJECT(dev));
> - object_unref(OBJECT(dev));
> - return NULL;
> + goto err_del_dev;
> }
>
> dev->opts = opts;
> object_property_set_bool(OBJECT(dev), true, "realized", &err);
> - if (err != NULL) {
> - error_propagate(errp, err);
> - dev->opts = NULL;
> - object_unparent(OBJECT(dev));
> - object_unref(OBJECT(dev));
> - return NULL;
> + if (!err) {
> + return dev;
> }
typically the same error check pattern is used through out the function
so I'd not do inversion here. i.e. keep normal flow non-branched and jump on
error
if (err != NULL) {
goto err_del_dev;
}
return dev;
err_del_dev:
dev->opts = NULL;
> - return dev;
> + dev->opts = NULL;
> +
> +err_del_dev:
> + error_propagate(errp, err);
> + object_unparent(OBJECT(dev));
> + object_unref(OBJECT(dev));
> + return NULL;
> }
>
>
