On Thu, Nov 7, 2024 at 1:52 PM Dorinda Bassey <dbas...@redhat.com> wrote:
>
> In `virtio_add_resource` function, the UUID used as a key for
> `g_hash_table_insert` was temporary, which could lead to
> invalid lookups when accessed later. This patch ensures that
> the UUID remains valid by duplicating it into a newly allocated
> memory space (persistent_uuid). The value is then inserted into
> the hash table with this persistent UUID key to ensure that the
> key stored in the hash table remains valid as long as the hash
> table entry exists.
>
> Signed-off-by: Dorinda Bassey <dbas...@redhat.com>
> ---
> hw/display/virtio-dmabuf.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
> index 3dba4577ca7..4353970bc87 100644
> --- a/hw/display/virtio-dmabuf.c
> +++ b/hw/display/virtio-dmabuf.c
> @@ -39,7 +39,12 @@ static bool virtio_add_resource(QemuUUID *uuid,
> VirtioSharedObject *value)
> g_free);
> }
> if (g_hash_table_lookup(resource_uuids, uuid) == NULL) {
> - g_hash_table_insert(resource_uuids, uuid, value);
> + QemuUUID *persistent_uuid = g_memdup2(uuid, sizeof(QemuUUID));
> + if (persistent_uuid == NULL) {
> + result = false;
> + } else {
> + g_hash_table_insert(resource_uuids, persistent_uuid, value);
> + }
Reviewed-by: Albert Esteve <aest...@redhat.com>
The description of `virtio_add_*` functions in the header state:
"""
* Return: true if the UUID did not exist and the resource has been added,
* false if another resource with the same UUID already existed.
"""
I think it may be worth clarifying that it can also return false if
the resource failed
to be added.
> } else {
> result = false;
> }
> --
> 2.47.0
>
