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@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@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@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@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
Matthew Booth wrote:
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);
...
ACK for both of these.
Thanks. Pushed.
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.
On Tue, Sep 01, 2009 at 09:27:25AM +0100, Matthew Booth wrote:
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.
The main loop implementation needs to go, but some of these function pointers would remain (eg. abort_cb which is required if anyone is mad enough to replace the default action on malloc failure). Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
participants (3)
-
Jim Meyering -
Matthew Booth -
Richard W.M. Jones