On 2024/06/03 14:32, Dmitry Osipenko wrote:
On 6/2/24 08:45, Akihiko Odaki wrote:
...
+ case HOSTMEM_MR_FINISH_UNMAPPING:
+ ret = virgl_renderer_resource_unmap(res->base.resource_id);
+ if (ret) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: failed to unmap virgl resource: %s\n",
+ __func__, strerror(-ret));
+ return ret;
+ }
+ res->mr = NULL;
+ g_free(vmr);
+ break;
+ case HOSTMEM_MR_UNMAPPING:
+ *cmd_suspended = true;
This code path should be unreachable since the command processing is
blocked while unmapping.
Will change to abort()
I don't think abort() call is needed here. You can just do what you do
for HOSTMEM_MR_MAPPED; the reference counter will automatically catch
the double-free condition and abort.
+ if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries,
sizeof(cblob),
+ cmd, &res->base.addrs,
+ &res->base.iov,
&res->base.iov_cnt);
+ if (!ret) {
+ g_free(res);
As noted for an earlier version:
Use g_autofree instead of writing duplicate g_free() calls. See
docs/devel/style.rst for details.
The g_autofree isn't appropriate for this code. It's intended to be used
if you allocate a tmp variable that should be freed in all code paths.
This is not the case here, the res variable isn't temporal and shall not
be freed on success.
You can assign NULL to res after QTAILQ_INSERT_HEAD(). See
rutabaga_cmd_resource_create_blob() for example.
Usually g_steal_pointer() should be used in such a situation but
unfortunately it is not possible in this case due to how
QTAILQ_INSERT_HEAD() macro is implemented.
