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()
>> + 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.
--
Best regards,
Dmitry
