Libguestfs - Aug 2009 - two small patches to appease clang/llvm static analysis

I ran libguestfs through llvm+clang, today,
[http://clang.llvm.org/StaticAnalysis.html]

It found only two things worth changing -- neither is a real problems.

Adding the noreturn makes it so the tool understands line 541 of
guestfs.c is truly reachable only for non-NULL "p":

  540	 if (!p) g->abort_cb ();
  541	 memcpy (p, ptr, size);

>From 41f8b506924243d4fd7570913fbbbd9f8040e11f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Mon, 31 Aug 2009 19:51:46 +0200
Subject: [PATCH libguestfs 1/2] maint: guestfs.c: remove unnecessary
initialization

* src/guestfs.c (guestfs__receive_file_sync): Don't set "r",
only to ignore it.
---
 src/guestfs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c
index 145f0f3..20afb63 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -2413,7 +2413,7 @@ guestfs__receive_file_sync (guestfs_h *g, const char
*filename)
     return -1;
   }

-  while ((r = receive_file_data_sync (g, NULL, NULL)) > 0)
+  while (receive_file_data_sync (g, NULL, NULL) > 0)
     ;				/* just discard it */

   return -1;
--
1.6.4.2.384.g5fc62

>From 97ef6a82ed32404b751a23b6658e0f9c7ab3298c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Mon, 31 Aug 2009 20:29:08 +0200
Subject: [PATCH libguestfs 2/2] maint: guestfs.c: avoid warning about possible
NULL deref from llvm/clang

* src/guestfs.h (guestfs_abort_cb): Declare with attribute noreturn.
---
 src/guestfs.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/guestfs.h b/src/guestfs.h
index f2e108b..6412a53 100644
--- a/src/guestfs.h
+++ b/src/guestfs.h
@@ -41,7 +41,7 @@ extern void guestfs_close (guestfs_h *g);
 extern const char *guestfs_last_error (guestfs_h *g);

 typedef void (*guestfs_error_handler_cb) (guestfs_h *g, void *data, const char
*msg);
-typedef void (*guestfs_abort_cb) (void);
+typedef void (*guestfs_abort_cb) (void) __attribute__((__noreturn__));

 extern void guestfs_set_error_handler (guestfs_h *g, guestfs_error_handler_cb
cb, void *data);
 extern guestfs_error_handler_cb guestfs_get_error_handler (guestfs_h *g, void
**data_rtn);
--
1.6.4.2.384.g5fc62

On 31/08/09 20:45, Jim Meyering wrote:
> I ran libguestfs through llvm+clang, today,
> [http://clang.llvm.org/StaticAnalysis.html]
>
> It found only two things worth changing -- neither is a real problems.
>
> Adding the noreturn makes it so the tool understands line 541 of
> guestfs.c is truly reachable only for non-NULL "p":
>
>    540	 if (!p) g->abort_cb ();
>    541	 memcpy (p, ptr, size);
>
>
>> From 41f8b506924243d4fd7570913fbbbd9f8040e11f Mon Sep 17 00:00:00 2001
> From: Jim Meyering<meyering at redhat.com>
> Date: Mon, 31 Aug 2009 19:51:46 +0200
> Subject: [PATCH libguestfs 1/2] maint: guestfs.c: remove unnecessary
initialization
>
> * src/guestfs.c (guestfs__receive_file_sync): Don't set "r",
> only to ignore it.
> ---
>   src/guestfs.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/guestfs.c b/src/guestfs.c
> index 145f0f3..20afb63 100644
> --- a/src/guestfs.c
> +++ b/src/guestfs.c
> @@ -2413,7 +2413,7 @@ guestfs__receive_file_sync (guestfs_h *g, const char
*filename)
>       return -1;
>     }
>
> -  while ((r = receive_file_data_sync (g, NULL, NULL))>  0)
> +  while (receive_file_data_sync (g, NULL, NULL)>  0)
>       ;				/* just discard it */
>
>     return -1;
> --
> 1.6.4.2.384.g5fc62
>
>
>> From 97ef6a82ed32404b751a23b6658e0f9c7ab3298c Mon Sep 17 00:00:00 2001
> From: Jim Meyering<meyering at redhat.com>
> Date: Mon, 31 Aug 2009 20:29:08 +0200
> Subject: [PATCH libguestfs 2/2] maint: guestfs.c: avoid warning about
possible NULL deref from llvm/clang
>
> * src/guestfs.h (guestfs_abort_cb): Declare with attribute noreturn.
> ---
>   src/guestfs.h |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/guestfs.h b/src/guestfs.h
> index f2e108b..6412a53 100644
> --- a/src/guestfs.h
> +++ b/src/guestfs.h
> @@ -41,7 +41,7 @@ extern void guestfs_close (guestfs_h *g);
>   extern const char *guestfs_last_error (guestfs_h *g);
>
>   typedef void (*guestfs_error_handler_cb) (guestfs_h *g, void *data, const
char *msg);
> -typedef void (*guestfs_abort_cb) (void);
> +typedef void (*guestfs_abort_cb) (void) __attribute__((__noreturn__));
>
>   extern void guestfs_set_error_handler (guestfs_h *g,
guestfs_error_handler_cb cb, void *data);
>   extern guestfs_error_handler_cb guestfs_get_error_handler (guestfs_h *g,
void **data_rtn);
> --
> 1.6.4.2.384.g5fc62
ACK for both of these.

One for Rich when he gets back is whether these function pointers remain 
worthwhile. IIRC they were intended to be used when integrating with an 
external main loop, which I believe has fallen out of favour. I don't 
see any of these functions being replaced, so calling them directly 
might simplify things a little.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490