On 11/18/25 08:45, Markus Armbruster wrote: > Dmitry Osipenko <[email protected]> writes: > >> On 11/17/25 16:22, Markus Armbruster wrote: >>> Dmitry Osipenko <[email protected]> writes: >>> >>>> On 11/17/25 13:51, Honglei Huang wrote: >>>>> diff --git a/hw/display/virtio-gpu-rutabaga.c >>>>> b/hw/display/virtio-gpu-rutabaga.c >>>>> index ed5ae52acb..ea2928b706 100644 >>>>> --- a/hw/display/virtio-gpu-rutabaga.c >>>>> +++ b/hw/display/virtio-gpu-rutabaga.c >>>>> @@ -466,7 +466,7 @@ rutabaga_cmd_attach_backing(VirtIOGPU *g, struct >>>>> virtio_gpu_ctrl_command *cmd) >>>>> >>>>> ret = virtio_gpu_create_mapping_iov(g, att_rb.nr_entries, >>>>> sizeof(att_rb), >>>>> cmd, NULL, &res->iov, >>>>> &res->iov_cnt); >>>>> - CHECK(!ret, cmd); >>>>> + CHECK(ret >= 0, cmd); >>>> >>>> virtio_gpu_create_mapping_iov() doesn't return positive values, don't >>>> see how this change improves anything. You now saying that ret > 0 is >>>> okay, while it shall never happen. >>> >>> Please see >>> >>> Subject: Re: [PATCH] virtio-gpu-virgl: fix error handling in >>> virgl_cmd_resource_create_blob >>> Date: Mon, 17 Nov 2025 08:49:42 +0100 >>> Message-ID: <[email protected]> >>> https://lore.kernel.org/qemu-devel/[email protected]/ >> >> It's a rather common bug when errno isn't negated by mistake and a >> positive error code is returned. Ignoring positive values when they >> aren't expected opens door to unnecessary problems, IMO. > > No convention can avoid *all* possible problems there. I know which one > has created more grief for *me*. Your experience may be different. > > virtio_gpu_create_mapping_iov() returns -1 on error, 0 on success. > We could change it to return false on error, true on success.
True/false are also often confusing. As was written in the other email, this variant is now good to me. In the end I'm glad for having this discussion, it will help with further improvement of virtio-gpu code. -- Best regards, Dmitry
