| Message ID | 20200821084945.5264-1-lizhijian@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-gpu: fix unmap the already mapped items | expand |
On Fri, Aug 21, 2020 at 04:49:45PM +0800, Li Zhijian wrote: > we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider > to unmap the 'i'th item as well when the 'i'th item is not nil > > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > --- > hw/display/virtio-gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 5f0dd7c150..1f777e43ff 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -656,7 +656,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g, > qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for" > " resource %d element %d\n", > __func__, ab->resource_id, i); > - virtio_gpu_cleanup_mapping_iov(g, *iov, i); > + virtio_gpu_cleanup_mapping_iov(g, *iov, i + !!(*iov)[i].iov_base); Cute trick, but the code should be readable without having to dig out the commit message which explains it. Can we have something simpler along the lines of "if (iov_base) { i++; /* cleanup partial map */ }" please? thanks, Gerd
On 8/21/20 6:07 PM, Gerd Hoffmann wrote: > On Fri, Aug 21, 2020 at 04:49:45PM +0800, Li Zhijian wrote: >> we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider >> to unmap the 'i'th item as well when the 'i'th item is not nil >> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- >> hw/display/virtio-gpu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c >> index 5f0dd7c150..1f777e43ff 100644 >> --- a/hw/display/virtio-gpu.c >> +++ b/hw/display/virtio-gpu.c >> @@ -656,7 +656,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g, >> qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for" >> " resource %d element %d\n", >> __func__, ab->resource_id, i); >> - virtio_gpu_cleanup_mapping_iov(g, *iov, i); >> + virtio_gpu_cleanup_mapping_iov(g, *iov, i + !!(*iov)[i].iov_base); > Cute trick, but the code should be readable without having to dig out > the commit message which explains it. Can we have something simpler > along the lines of "if (iov_base) { i++; /* cleanup partial map */ }" > please? make sense ! Thanks Zhijian > > thanks, > Gerd > > >
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 5f0dd7c150..1f777e43ff 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -656,7 +656,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g, qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for" " resource %d element %d\n", __func__, ab->resource_id, i); - virtio_gpu_cleanup_mapping_iov(g, *iov, i); + virtio_gpu_cleanup_mapping_iov(g, *iov, i + !!(*iov)[i].iov_base); g_free(ents); *iov = NULL; if (addr) {
we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider to unmap the 'i'th item as well when the 'i'th item is not nil Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> --- hw/display/virtio-gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)