Re: [PATCH v13 11/13] virtio-gpu: Handle resource blob commands

[Dmitry Osipenko]

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