Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [RFC] hw/usb/xen-usb.c: Pass struct usbback_req* to usbback_packet_complete()
@ 2020-03-23 16:43 Peter Maydell
  2020-03-25  9:48 ` Gerd Hoffmann
  0 siblings, 1 reply; 2+ messages in thread
From: Peter Maydell @ 2020-03-23 16:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anthony Perard, xen-devel, Stefano Stabellini, Gerd Hoffmann,
	Paul Durrant

The function usbback_packet_complete() currently takes a USBPacket*,
which must be a pointer to the packet field within a struct
usbback_req; the function uses container_of() to get the struct
usbback_req* given the USBPacket*.

This is unnecessarily confusing (and in particular it confuses the
Coverity Scan analysis, resulting in the false positive CID 1421919
where it thinks that we write off the end of the structure). Since
both callsites already have the pointer to the struct usbback_req,
just pass that in directly.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This is an RFC because:
 * I'm not very familiar with the Xen bits of QEMU
 * the main rationale here is to change something that's
   confusing Coverity -- the code as it stands isn't wrong
 * the only testing I've done is "make check"
Still, the change seems like a good thing to me as a human reader...

PS: QEMU's MAINTAINERS file stanza for Xen doesn't pick up
that this file is Xen related, so it could use an extra F: line.

 hw/usb/xen-usb.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 1fc2f32ce93..961190d0f78 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -347,13 +347,11 @@ static int32_t usbback_xlat_status(int status)
     return -ESHUTDOWN;
 }
 
-static void usbback_packet_complete(USBPacket *packet)
+static void usbback_packet_complete(struct usbback_req *usbback_req)
 {
-    struct usbback_req *usbback_req;
+    USBPacket *packet = &usbback_req->packet;
     int32_t status;
 
-    usbback_req = container_of(packet, struct usbback_req, packet);
-
     QTAILQ_REMOVE(&usbback_req->stub->submit_q, usbback_req, q);
 
     status = usbback_xlat_status(packet->status);
@@ -566,7 +564,7 @@ static void usbback_dispatch(struct usbback_req *usbback_req)
 
     usb_handle_packet(usbback_req->stub->dev, &usbback_req->packet);
     if (usbback_req->packet.status != USB_RET_ASYNC) {
-        usbback_packet_complete(&usbback_req->packet);
+        usbback_packet_complete(usbback_req);
     }
     return;
 
@@ -993,7 +991,7 @@ static void xen_bus_complete(USBPort *port, USBPacket *packet)
 
     usbif = usbback_req->usbif;
     TR_REQ(&usbif->xendev, "\n");
-    usbback_packet_complete(packet);
+    usbback_packet_complete(usbback_req);
 }
 
 static USBPortOps xen_usb_port_ops = {
-- 
2.20.1



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Xen-devel] [RFC] hw/usb/xen-usb.c: Pass struct usbback_req* to usbback_packet_complete()
  2020-03-23 16:43 [Xen-devel] [RFC] hw/usb/xen-usb.c: Pass struct usbback_req* to usbback_packet_complete() Peter Maydell
@ 2020-03-25  9:48 ` Gerd Hoffmann
  0 siblings, 0 replies; 2+ messages in thread
From: Gerd Hoffmann @ 2020-03-25  9:48 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Anthony Perard, xen-devel, Stefano Stabellini, qemu-devel, Paul Durrant

On Mon, Mar 23, 2020 at 04:43:18PM +0000, Peter Maydell wrote:
> The function usbback_packet_complete() currently takes a USBPacket*,
> which must be a pointer to the packet field within a struct
> usbback_req; the function uses container_of() to get the struct
> usbback_req* given the USBPacket*.
> 
> This is unnecessarily confusing (and in particular it confuses the
> Coverity Scan analysis, resulting in the false positive CID 1421919
> where it thinks that we write off the end of the structure). Since
> both callsites already have the pointer to the struct usbback_req,
> just pass that in directly.

Looks sane from usb point of view.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23 16:43 [Xen-devel] [RFC] hw/usb/xen-usb.c: Pass struct usbback_req* to usbback_packet_complete() Peter Maydell
2020-03-25  9:48 ` Gerd Hoffmann

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git