On Wed, Jan 06, 2010 at 09:55:23PM +0100, Jim Meyering wrote:
Daniel Veillard wrote:
On Wed, Jan 06, 2010 at 12:46:11PM +0100, Jim Meyering wrote:
As the log says, once we've dereferenced it, there's no point in comparing to NULL.
From 463eaf1027a154e71839a67eca85b3ada8b817ff Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 6 Jan 2010 12:45:07 +0100 Subject: [PATCH] don't test "res == NULL" after we've already dereferenced it
* src/xen/proxy_internal.c (xenProxyCommand): It is known to be non-NULL at that point, so remove the "ret == NULL" guard, and instead add an assert(ret != NULL), in case future code changes cause the condition to becomes false. Include <assert.h>. --- src/xen/proxy_internal.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c index ec4522b..42b6e91 100644 --- a/src/xen/proxy_internal.c +++ b/src/xen/proxy_internal.c @@ -1,7 +1,7 @@ /* * proxy_client.c: client side of the communication with the libvirt proxy. * - * Copyright (C) 2006, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006, 2008, 2009, 2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -11,6 +11,7 @@ #include <config.h>
#include <stdio.h> +#include <assert.h> #include <stdlib.h> #include <unistd.h> #include <errno.h> @@ -444,7 +445,8 @@ retry: /* * do more checks on the incoming packet. */ - if ((res == NULL) || (res->version != PROXY_PROTO_VERSION) || + assert (res != NULL); + if ((res->version != PROXY_PROTO_VERSION) || (res->len < sizeof(virProxyPacket))) { virProxyError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Communication error with proxy: malformed packet\n"));
I'm not too fond of adding an assert, res should not be null there or we should already have crashed.
ACK to the change without the new assert line.
Considering that this is in the daemon and that "bad things" have been known to happen via NULL derefs, some would argue that an assertion failure is preferable.
No, this code is the client side of the Xen proxy implementation, that is never used within daemon context. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|