[PATCH] Re: [libvirt] three more clang-inspired dead-store-fixing patches
Daniel Veillard
veillard at redhat.com
Fri Sep 4 14:38:57 UTC 2009
On Fri, Sep 04, 2009 at 11:34:09AM +0200, Jim Meyering wrote:
> Jim Meyering wrote:
> >>From a2d03c987bb724283207dbeef873178c08a6c4c5 Mon Sep 17 00:00:00 2001
> > From: Jim Meyering <meyering at redhat.com>
> > Date: Thu, 3 Sep 2009 18:14:48 +0200
> > Subject: [PATCH 1/3] storage_backend_logical.c: appease clang: remove useless increment
> ...
> >>From b72ed1ba5d5e6444f000fd9be706a51a90dd2292 Mon Sep 17 00:00:00 2001
> > From: Jim Meyering <meyering at redhat.com>
> > Date: Thu, 3 Sep 2009 18:23:51 +0200
> > Subject: [PATCH 2/3] openvz_conf.c: Remove dead store to copy_fd
> ...
> >>From a8d2eca13635578084e3c2cc2093562d9a8fcfed Mon Sep 17 00:00:00 2001
> > From: Jim Meyering <meyering at redhat.com>
> > Date: Thu, 3 Sep 2009 18:25:03 +0200
> > Subject: [PATCH 3/3] eventtest.c: detect write failure and avoid dead stores
> >
> > * tests/eventtest.c (mymain): Exit nonzero upon write failure.
> > This also avoids several dead stores of the form ret = safewrite...
> ...
>
> Here's a slight adjustment.
> That third patch missed one of these:
>
> - ret = safewrite(handles[1].pipeFD[1], &one, 1);
> + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1)
> + return EXIT_FAILURE;
>
> It's corrected below:
>
> >From 7cba5226a7a1f1626263e5f35a0e342c7c4d7151 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Thu, 3 Sep 2009 18:14:48 +0200
> Subject: [PATCH 1/3] storage_backend_logical.c: appease clang: remove useless increment
>
> * src/storage_backend_logical.c (virStorageBackendLogicalBuildPool):
> Don't increment "n" when we won't use the result.
> ---
> src/storage_backend_logical.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
> index bc40dd7..43117e8 100644
> --- a/src/storage_backend_logical.c
> +++ b/src/storage_backend_logical.c
> @@ -437,7 +437,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn,
> goto cleanup;
> }
>
> - vgargv[n++] = NULL;
> + vgargv[n] = NULL;
>
> /* Now create the volume group itself */
> if (virRun(conn, vgargv, NULL) < 0)
> --
> 1.6.4.2.409.g85dc3
ACK
>
> >From 340fe0e28a613b3c15f3c64cb17b4ba64e293128 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Thu, 3 Sep 2009 18:23:51 +0200
> Subject: [PATCH 2/3] openvz_conf.c: Remove dead store to copy_fd
>
> * src/openvz_conf.c (openvz_copyfile): Remove unused assignment.
> ---
> src/openvz_conf.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/src/openvz_conf.c b/src/openvz_conf.c
> index 41c6684..2f0471c 100644
> --- a/src/openvz_conf.c
> +++ b/src/openvz_conf.c
> @@ -692,7 +692,6 @@ openvz_copyfile(char* from_path, char* to_path)
> fd = -1;
> if (close(copy_fd) < 0)
> goto error;
> - copy_fd = -1;
>
> return 0;
>
> --
> 1.6.4.2.409.g85dc3
ACK, though I would expect the compiler is able to optimize this :-)
> >From e6fa6de8689fda71920cf0918a1022230d2cad70 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Thu, 3 Sep 2009 18:25:03 +0200
> Subject: [PATCH 3/3] eventtest.c: detect write failure and avoid dead stores
>
> * tests/eventtest.c (mymain): Exit nonzero upon write failure.
> This also avoids several dead stores of the form ret = safewrite...
> ---
> tests/eventtest.c | 18 +++++++++++-------
> 1 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/tests/eventtest.c b/tests/eventtest.c
> index 68ac2fc..da34388 100644
> --- a/tests/eventtest.c
> +++ b/tests/eventtest.c
> @@ -248,7 +248,6 @@ resetAll(void)
> static int
> mymain(int argc, char **argv)
> {
> - int ret = 0;
> char *progname;
> int i;
> pthread_t eventThread;
> @@ -304,7 +303,8 @@ mymain(int argc, char **argv)
> /* First time, is easy - just try triggering one of our
> * registered handles */
> startJob("Simple write", &test);
> - ret = safewrite(handles[1].pipeFD[1], &one, 1);
> + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1)
> + return EXIT_FAILURE;
> if (finishJob(1, -1) != EXIT_SUCCESS)
> return EXIT_FAILURE;
>
> @@ -314,7 +314,8 @@ mymain(int argc, char **argv)
> * try triggering another handle */
> virEventRemoveHandleImpl(handles[0].watch);
> startJob("Deleted before poll", &test);
> - ret = safewrite(handles[1].pipeFD[1], &one, 1);
> + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1)
> + return EXIT_FAILURE;
> if (finishJob(1, -1) != EXIT_SUCCESS)
> return EXIT_FAILURE;
>
> @@ -346,8 +347,9 @@ mymain(int argc, char **argv)
> * about */
> startJob("Deleted during dispatch", &test);
> handles[2].delete = handles[3].watch;
> - ret = safewrite(handles[2].pipeFD[1], &one, 1);
> - ret = safewrite(handles[3].pipeFD[1], &one, 1);
> + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1
> + || safewrite(handles[3].pipeFD[1], &one, 1) != 1)
<nitpick> I prefer || at end of previous line and superfluous
but nicer to read extra parenthesis :-) </nitpick>
> + return EXIT_FAILURE;
> if (finishJob(2, -1) != EXIT_SUCCESS)
> return EXIT_FAILURE;
>
> @@ -356,7 +358,8 @@ mymain(int argc, char **argv)
> /* Extreme fun, lets delete ourselves during dispatch */
> startJob("Deleted during dispatch", &test);
> handles[2].delete = handles[2].watch;
> - ret = safewrite(handles[2].pipeFD[1], &one, 1);
> + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1)
> + return EXIT_FAILURE;
> if (finishJob(2, -1) != EXIT_SUCCESS)
> return EXIT_FAILURE;
>
> @@ -446,7 +449,8 @@ mymain(int argc, char **argv)
> testPipeReader,
> &handles[1], NULL);
> startJob("Write duplicate", &test);
> - ret = safewrite(handles[1].pipeFD[1], &one, 1);
> + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1)
> + return EXIT_FAILURE;
> if (finishJob(1, -1) != EXIT_SUCCESS)
> return EXIT_FAILURE;
>
ACK,
thanks !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list