Re: [libvirt] [PATCH] xen_hypervisor.c: avoid NULL deref for NULL domain argument

> >> Subject: [PATCH] xen_hypervisor.c: avoid NULL deref for NULL domain argument > >> > >> * src/xen/xen_hypervisor.c (xenHypervisorGetVcpus): Don't attempt > >> to diagnose an unlikely NULL-domain or NULL-domain->conn error.

> >> @@ -3475,11 +3475,8 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, > >> virVcpuInfoPtr ipt; > >> int nbinfo, i; > >> > >> - if (domain == NULL || domain->conn == NULL) { > >> - virXenErrorFunc (domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__, > >> - "invalid argument", 0); > >> + if (domain == NULL || domain->conn == NULL) > >> return -1; > >> - } > > > > I'd rather we just got rid of that check completely - its a left > > over from a time when Xen was the only driver & these entry points > > needed to do full checking. Now all mandatory parameters are checked > > in the previous libvirt.c layer. > > Here's an additional patch, to eliminate all of the "domain == NULL" > tests. I want to keep this global "clean-up" patch separate from > the above bug-fixing one. > > I'm less confident about removing the daomin->conn tests, > and would be inclined to leave them as-is, or use an assert, if you > want to remove them. If we also remove the daomin->conn == NULL tests, > an added "assert" is the best way to keep clang/coverity from > complaining about a possible NULL-dereference. > > From 9e6f7ca7a0dfa6b220e598d04ca40d33e67feb22 Mon Sep 17 00:00:00 2001 > From: Jim Meyering <meyering(a)redhat.com&gt; > Date: Wed, 27 Jan 2010 13:34:03 +0100 > Subject: [PATCH] xen_hypervisor.c: remove all "domain == NULL" tests, ... > > * src/xen/xen_hypervisor.c: Remove all "domain == NULL" tests. > * src/xen/xen_hypervisor.h: Instead, use ATTRIBUTE_NONNULL to > mark each "domain" parameter as "known always to be non-NULL".

ACK