[PATCH] A few Coverity fixes
Volker Lendecke
vl at samba.org
Fri Oct 14 11:07:46 UTC 2016
Hi!
Review appreciated!
Thanks, Volker
-------------- next part --------------
>From 6de5a078c761d817ec1bb88d0ebd3b635026328c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Oct 2016 12:12:28 +0200
Subject: [PATCH 1/6] pthreadpool: Rearrange locks a bit
Coverity ID 1373624 says we have a deadlock between pthreadpool_prepare and
pthreadpool_destroy. Coverity somehow misses that pthreadpool_free unlocks
pool->mutex, so I think this is a false positive. Nevertheless this re-arranges
the code a bit for more clarity, hoping that Coverity now can better track the
locks and unlocks. Also, the human reader might have to jump between routines a
bit less.
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/pthreadpool/pthreadpool.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/source3/lib/pthreadpool/pthreadpool.c b/source3/lib/pthreadpool/pthreadpool.c
index a1eb924..f34e219 100644
--- a/source3/lib/pthreadpool/pthreadpool.c
+++ b/source3/lib/pthreadpool/pthreadpool.c
@@ -234,9 +234,6 @@ static int pthreadpool_free(struct pthreadpool *pool)
{
int ret, ret1;
- ret = pthread_mutex_unlock(&pool->mutex);
- assert(ret == 0);
-
ret = pthread_mutex_destroy(&pool->mutex);
ret1 = pthread_cond_destroy(&pool->condvar);
@@ -275,23 +272,26 @@ int pthreadpool_destroy(struct pthreadpool *pool)
return ret;
}
- if (pool->num_threads == 0) {
- ret = pthreadpool_free(pool);
- return ret;
- }
-
if (pool->shutdown) {
ret = pthread_mutex_unlock(&pool->mutex);
assert(ret == 0);
return EBUSY;
}
+ pool->shutdown = true;
+
+ if (pool->num_threads == 0) {
+ ret = pthread_mutex_unlock(&pool->mutex);
+ assert(ret == 0);
+
+ ret = pthreadpool_free(pool);
+ return ret;
+ }
+
/*
* We have active threads, tell them to finish.
*/
- pool->shutdown = true;
-
ret = pthread_cond_broadcast(&pool->condvar);
ret1 = pthread_mutex_unlock(&pool->mutex);
@@ -308,16 +308,18 @@ int pthreadpool_destroy(struct pthreadpool *pool)
static void pthreadpool_server_exit(struct pthreadpool *pool)
{
int ret;
+ bool free_it;
pool->num_threads -= 1;
- if (pool->shutdown && (pool->num_threads == 0)) {
- pthreadpool_free(pool);
- return;
- }
+ free_it = (pool->shutdown && (pool->num_threads == 0));
ret = pthread_mutex_unlock(&pool->mutex);
assert(ret == 0);
+
+ if (free_it) {
+ pthreadpool_free(pool);
+ }
}
static bool pthreadpool_get_job(struct pthreadpool *p,
--
2.1.4
>From bfdef700493a3ccfc3f598744973b1a672801ee1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Oct 2016 12:18:11 +0200
Subject: [PATCH 2/6] talloc: Fix CID 1373621 Unchecked return value
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/talloc/testsuite.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index 8fe5d90..cff443c 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -34,6 +34,12 @@
#include <unistd.h>
#include <sys/wait.h>
+#ifdef NDEBUG
+#undef NDEBUG
+#endif
+
+#include <assert.h>
+
#include "talloc_testsuite.h"
static struct timeval private_timeval_current(void)
@@ -1750,7 +1756,8 @@ static void *thread_fn(void *arg)
ret = pthread_cond_wait(&condvar, &mtx);
if (ret != 0) {
talloc_free(top_ctx);
- pthread_mutex_unlock(&mtx);
+ ret = pthread_mutex_unlock(&mtx);
+ assert(ret == 0);
return NULL;
}
}
@@ -1760,7 +1767,8 @@ static void *thread_fn(void *arg)
/* Tell the main thread it's ready for pickup. */
pthread_cond_broadcast(&condvar);
- pthread_mutex_unlock(&mtx);
+ ret = pthread_mutex_unlock(&mtx);
+ assert(ret == 0);
talloc_free(top_ctx);
return NULL;
--
2.1.4
>From eac8fc6802d624f9a07949896e662ad5eabe1d63 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Oct 2016 12:20:00 +0200
Subject: [PATCH 3/6] pthreadpool: Fix CID 1373620 Unchecked return value from
library
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/pthreadpool/tests.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/source3/lib/pthreadpool/tests.c b/source3/lib/pthreadpool/tests.c
index 4d211f2..c4d2e6a 100644
--- a/source3/lib/pthreadpool/tests.c
+++ b/source3/lib/pthreadpool/tests.c
@@ -132,7 +132,9 @@ static int test_busydestroy(void)
pfd.fd = pthreadpool_pipe_signal_fd(p);
pfd.events = POLLIN|POLLERR;
- poll(&pfd, 1, -1);
+ do {
+ ret = poll(&pfd, 1, -1);
+ } while ((ret == -1) && (errno == EINTR));
ret = pthreadpool_pipe_finished_jobs(p, &jobid, 1);
if (ret < 0) {
--
2.1.4
>From 47c5e37d50c3b3b41a41cf79827a790de1b19b0d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Oct 2016 12:21:36 +0200
Subject: [PATCH 4/6] messaging: Fix CID 1373625 Unused value
Hmm. I wonder how that cut&paste happened...
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source4/lib/messaging/tests/messaging.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/source4/lib/messaging/tests/messaging.c b/source4/lib/messaging/tests/messaging.c
index 783850a..ba58978 100644
--- a/source4/lib/messaging/tests/messaging.c
+++ b/source4/lib/messaging/tests/messaging.c
@@ -171,6 +171,7 @@ static bool test_messaging_overflow(struct torture_context *tctx)
} while ((nwritten == -1) && (errno == EINTR));
ret = close(down_pipe[1]);
+ torture_assert(tctx, ret == 0, "close failed");
do {
nread = read(down_pipe[0], &c, 1);
--
2.1.4
>From 827ef505d3713f3c7a673afed2577791692ae9d2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Oct 2016 12:25:39 +0200
Subject: [PATCH 5/6] messaging: Fix CID 1373622 Extra high-order bits
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/lib/messages_dgm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/lib/messages_dgm.c b/source3/lib/messages_dgm.c
index 6bdd589..49b3903 100644
--- a/source3/lib/messages_dgm.c
+++ b/source3/lib/messages_dgm.c
@@ -1133,7 +1133,7 @@ static void messaging_dgm_read_handler(struct tevent_context *ev,
msghdr_prep_recv_fds(&msg, msgbuf, msgbufsize, INT8_MAX);
#ifdef MSG_CMSG_CLOEXEC
- flags |= MSG_CMSG_CLOEXEC;
+ msg.msg_flags |= MSG_CMSG_CLOEXEC;
#endif
received = recvmsg(ctx->sock, &msg, 0);
--
2.1.4
>From 9116c9de3b08529950de65ca22518afa35ca728f Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Oct 2016 12:27:56 +0200
Subject: [PATCH 6/6] talloc: Fix CID 1373619 Unchecked return value
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/talloc/testsuite.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index cff443c..7f98f4b 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -1839,8 +1839,8 @@ static bool test_pthread_talloc_passing(void)
printf("pthread_cond_wait %d failed (%d)\n", i,
ret);
talloc_free(mem_ctx);
- pthread_mutex_unlock(&mtx);
- return false;
+ ret = pthread_mutex_unlock(&mtx);
+ assert(ret == 0);
}
}
@@ -1849,7 +1849,8 @@ static bool test_pthread_talloc_passing(void)
/* Tell the sub-threads we're ready for another. */
pthread_cond_broadcast(&condvar);
- pthread_mutex_unlock(&mtx);
+ ret = pthread_mutex_unlock(&mtx);
+ assert(ret == 0);
}
CHECK_SIZE("pthread_talloc_passing", mem_ctx, NUM_THREADS * 100);
--
2.1.4
More information about the samba-technical
mailing list