On 10/11/21 23:00, Eric Blake wrote:
On Fri, Oct 08, 2021 at 03:34:36PM +0200, Kevin Wolf wrote:
From: Damien Hedde <damien.hedde@xxxxxxxxxxxxx>
qdev_set_id() is mostly used when the user adds a device (using
-device cli option or device_add qmp command). This commit adds
an error parameter to handle the case where the given id is
already taken.
Also document the function and add a return value in order to
be able to capture success/failure: the function now returns the
id in case of success, or NULL in case of failure.
The commit modifies the 2 calling places (qdev-monitor and
xen-legacy-backend) to add the error object parameter.
Note that the id is, right now, guaranteed to be unique because
all ids came from the "device" QemuOptsList where the id is used
as key. This addition is a preparation for a future commit which
will relax the uniqueness.
Signed-off-by: Damien Hedde <damien.hedde@xxxxxxxxxxxxx>
Signed-off-by: Kevin Wolf <kwolf@xxxxxxxxxx>
---
+ } else {
+ error_setg(errp, "Duplicate device ID '%s'", id);
+ return NULL;
id is not freed on this error path...
DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
@@ -691,7 +703,13 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
}
}
- qdev_set_id(dev, g_strdup(qemu_opts_id(opts)));
+ /*
+ * set dev's parent and register its id.
+ * If it fails it means the id is already taken.
+ */
+ if (!qdev_set_id(dev, g_strdup(qemu_opts_id(opts)), errp)) {
+ goto err_del_dev;
...nor on this, which means the g_strdup() leaks on failure.
Since we strdup the id just before calling qdev_set_id.
Maybe we should do the the strdup in qdev_set_id (and free things on
error there too). It seems simplier than freeing things on the callee
side just in case of an error.
Damien
