Re: [libvirt] [PATCH 2/3] Add RPC implementation for virDomainOpenGraphicsFd
On 08/25/2014 12:22 PM, Ján Tomko wrote:
---
daemon/remote.c | 42 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 39 +++++++++++++++++++++++++++++++++++++++
src/remote/remote_protocol.x | 15 ++++++++++++++-
src/rpc/virnetmessage.c | 26 ++++++++++++++++++++++++++
src/rpc/virnetmessage.h | 3 +++
5 files changed, 124 insertions(+), 1 deletion(-)
static int
+remoteDomainOpenGraphicsFD(virDomainPtr dom,
+ unsigned int idx,
+ int *fd,
+ unsigned int flags)
+{
+ int rv = -1;
+ remote_domain_open_graphics_args args;
+ struct private_data *priv = dom->conn->privateData;
+ int *fdout = NULL;
NULL here...
+ size_t fdoutlen = 0;
+
+ remoteDriverLock(priv);
+
+ make_nonnull_domain(&args.dom, dom);
+ args.idx = idx;
+ args.flags = flags;
+
+ if (callFull(dom->conn, priv, 0,
+ NULL, 0,
+ &fdout, &fdoutlen,
+ REMOTE_PROC_DOMAIN_OPEN_GRAPHICS_FD,
+ (xdrproc_t) xdr_remote_domain_open_graphics_fd_args, (char *)
&args,
+ (xdrproc_t) xdr_void, NULL) == -1)
+ goto done;
...non-NULL here, which means callFull allocated it...
+
+ /* TODO: Check fdoutlen */
+ *fd = fdout[0];
+
+ rv = 0;
+
+ done:
+ remoteDriverUnlock(priv);
+
+ return rv;
...but you leak the memory. Valgrind will complain. Furthermore, in
the normal error case, the callFull leaves fdoutlen as 0, so the error
message you checked in makes sense. But in the unusual error case of
callFull getting 2 fds from the other side, you are leaking those fds;
if we're going to declare error because fdoutlen != 1, then we should
also do a VIR_FORCE_CLOSE loop on the array. I've incorporated that
into my proposed followup patch:
https://www.redhat.com/archives/libvir-list/2014-August/msg01281.html
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)