[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] don't test "res == NULL" after we've already dereferenced it



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.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]