Download raw body.
rpki-client: add error checks for pthread_* calls
On Tue, Jun 24, 2025 at 10:06:18AM +0200, Theo Buehler wrote:
> On Tue, Jun 24, 2025 at 10:01:47AM +0200, Theo Buehler wrote:
> > > or
> > > errx(1, "pthread_create: %s", strerror(error));
> >
> > Alright. I'm not going to do errc myself and if anyone really wants it,
> > please go and fix portable first.
>
> Bah. Disregard. Need to use err.
>
This time with errx. I also added error checks for two cond_wait.
Some checks were pulled out of the if clauses since there's no
sensible wrapping possible.
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.168 cert.c
--- cert.c 24 Jun 2025 05:21:55 -0000 1.168
+++ cert.c 24 Jun 2025 07:48:39 -0000
@@ -1426,15 +1426,16 @@ auth_find(struct auth_tree *auths, int i
{
struct auth a, *f;
struct cert c;
+ int error;
c.certid = id;
a.cert = &c;
- if (pthread_rwlock_rdlock(&cert_lk) != 0)
- errx(1, "pthread_rwlock_rdlock");
+ if ((error = pthread_rwlock_rdlock(&cert_lk)) != 0)
+ errx(1, "pthread_rwlock_rdlock: %s", strerror(error));
f = RB_FIND(auth_tree, auths, &a);
- if (pthread_rwlock_unlock(&cert_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&cert_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
return f;
}
@@ -1443,13 +1444,14 @@ auth_insert(const char *fn, struct auth_
struct auth *issuer)
{
struct auth *na;
+ int error;
na = calloc(1, sizeof(*na));
if (na == NULL)
err(1, NULL);
- if (pthread_rwlock_wrlock(&cert_lk) != 0)
- errx(1, "pthread_rwlock_wrlock");
+ if ((error = pthread_rwlock_wrlock(&cert_lk)) != 0)
+ errx(1, "pthread_rwlock_wrlock: %s", strerror(error));
if (issuer == NULL) {
cert->certid = cert->talid;
} else {
@@ -1474,14 +1476,14 @@ auth_insert(const char *fn, struct auth_
if (RB_INSERT(auth_tree, auths, na) != NULL)
errx(1, "auth tree corrupted");
- if (pthread_rwlock_unlock(&cert_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&cert_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
return na;
fail:
- if (pthread_rwlock_unlock(&cert_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&cert_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
free(na);
return NULL;
}
Index: crl.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
diff -u -p -r1.46 crl.c
--- crl.c 23 Jun 2025 22:01:14 -0000 1.46
+++ crl.c 24 Jun 2025 07:39:57 -0000
@@ -305,6 +305,7 @@ struct crl *
crl_get(struct crl_tree *crlt, const struct auth *a)
{
struct crl find, *crl;
+ int error;
/* XXX - this should be removed, but filemode relies on it. */
if (a == NULL)
@@ -313,24 +314,24 @@ crl_get(struct crl_tree *crlt, const str
find.aki = a->cert->ski;
find.mftpath = a->cert->mft;
- if (pthread_rwlock_rdlock(&crl_lk) != 0)
- errx(1, "pthread_rwlock_rdlock");
+ if ((error = pthread_rwlock_rdlock(&crl_lk)) != 0)
+ errx(1, "pthread_rwlock_rdlock: %s", strerror(error));
crl = RB_FIND(crl_tree, crlt, &find);
- if (pthread_rwlock_unlock(&crl_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&crl_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
return crl;
}
int
crl_insert(struct crl_tree *crlt, struct crl *crl)
{
- int rv;
+ int error, rv;
- if (pthread_rwlock_wrlock(&crl_lk) != 0)
- errx(1, "pthread_rwlock_wrlock");
+ if ((error = pthread_rwlock_wrlock(&crl_lk)) != 0)
+ errx(1, "pthread_rwlock_wrlock: %s", strerror(error));
rv = RB_INSERT(crl_tree, crlt, crl) == NULL;
- if (pthread_rwlock_unlock(&crl_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&crl_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
return rv;
}
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
diff -u -p -r1.158 parser.c
--- parser.c 23 Jun 2025 22:01:14 -0000 1.158
+++ parser.c 24 Jun 2025 08:04:15 -0000
@@ -75,12 +75,13 @@ static struct parse_repo *
repo_get(unsigned int id)
{
struct parse_repo needle = { .id = id }, *r;
+ int error;
- if (pthread_rwlock_rdlock(&repos_lk) != 0)
- errx(1, "pthread_rwlock_rdlock");
+ if ((error = pthread_rwlock_rdlock(&repos_lk)) != 0)
+ errx(1, "pthread_rwlock_rdlock: %s", strerror(error));
r = RB_FIND(repo_tree, &repos, &needle);
- if (pthread_rwlock_unlock(&repos_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&repos_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
return r;
}
@@ -88,6 +89,7 @@ static void
repo_add(unsigned int id, char *path, char *validpath)
{
struct parse_repo *rp;
+ int error;
if ((rp = calloc(1, sizeof(*rp))) == NULL)
err(1, NULL);
@@ -99,12 +101,12 @@ repo_add(unsigned int id, char *path, ch
if ((rp->validpath = strdup(validpath)) == NULL)
err(1, NULL);
- if (pthread_rwlock_wrlock(&repos_lk) != 0)
- errx(1, "pthread_rwlock_wrlock");
+ if ((error = pthread_rwlock_wrlock(&repos_lk)) != 0)
+ errx(1, "pthread_rwlock_wrlock: %s", strerror(error));
if (RB_INSERT(repo_tree, &repos, rp) != NULL)
errx(1, "repository already added: id %d, %s", id, path);
- if (pthread_rwlock_unlock(&repos_lk) != 0)
- errx(1, "pthread_rwlock_unlock");
+ if ((error = pthread_rwlock_unlock(&repos_lk)) != 0)
+ errx(1, "pthread_rwlock_unlock: %s", strerror(error));
}
/*
@@ -1072,7 +1074,7 @@ parse_worker(void *arg)
struct ibufqueue *myq;
X509_STORE_CTX *ctx;
BN_CTX *bn_ctx;
- int n;
+ int error, n;
if ((ctx = X509_STORE_CTX_new()) == NULL)
err(1, "X509_STORE_CTX_new");
@@ -1082,10 +1084,14 @@ parse_worker(void *arg)
err(1, "ibufqueue_new");
while (!quit) {
- if (pthread_mutex_lock(&globalq_mtx) != 0)
- errx(1, "pthread_mutex_lock");
- while (TAILQ_EMPTY(&globalq) && !quit)
- pthread_cond_wait(&globalq_cond, &globalq_mtx);
+ if ((error = pthread_mutex_lock(&globalq_mtx)) != 0)
+ errx(1, "pthread_mutex_lock: %s", strerror(error));
+ while (TAILQ_EMPTY(&globalq) && !quit) {
+ error = pthread_cond_wait(&globalq_cond, &globalq_mtx);
+ if (error != 0)
+ errx(1, "pthread_cond_wait: %s",
+ strerror(error));
+ }
n = 0;
while ((entp = TAILQ_FIRST(&globalq)) != NULL) {
TAILQ_REMOVE(&globalq, entp, entries);
@@ -1093,23 +1099,30 @@ parse_worker(void *arg)
if (n++ > 16)
break;
}
- if (pthread_mutex_unlock(&globalq_mtx) != 0)
- errx(1, "pthread_mutex_unlock");
+ if ((error = pthread_mutex_unlock(&globalq_mtx)) != 0)
+ errx(1, "pthread_mutex_unlock: %s",
+ strerror(error));
if (n > 16) {
- if (pthread_cond_signal(&globalq_cond) != 0)
- errx(1, "pthread_cond_signal");
+ if ((error = pthread_cond_signal(&globalq_cond)) != 0)
+ errx(1, "pthread_cond_signal: %s",
+ strerror(error));
}
parse_entity(&q, myq, ctx, bn_ctx);
if (ibufq_queuelen(myq) > 0) {
- if (pthread_mutex_lock(&globalmsgq_mtx) != 0)
- errx(1, "pthread_mutex_lock");
+ if ((error = pthread_mutex_lock(&globalmsgq_mtx)) != 0)
+ errx(1, "pthread_mutex_lock: %s",
+ strerror(error));
ibufq_concat(globalmsgq, myq);
- if (pthread_cond_signal(&globalmsgq_cond) != 0)
- errx(1, "pthread_cond_signal");
- if (pthread_mutex_unlock(&globalmsgq_mtx) != 0)
- errx(1, "pthread_mutex_unlock");
+ error = pthread_cond_signal(&globalmsgq_cond);
+ if (error != 0)
+ errx(1, "pthread_cond_signal: %s",
+ strerror(error));
+ error = pthread_mutex_unlock(&globalmsgq_mtx);
+ if (error != 0)
+ errx(1, "pthread_mutex_unlock: %s",
+ strerror(error));
}
}
@@ -1124,21 +1137,30 @@ parse_writer(void *arg)
{
struct msgbuf *myq;
struct pollfd pfd;
+ int error;
if ((myq = msgbuf_new()) == NULL)
err(1, NULL);
pfd.fd = *(int *)arg;
while (!quit) {
if (msgbuf_queuelen(myq) == 0) {
- if (pthread_mutex_lock(&globalmsgq_mtx) != 0)
- errx(1, "pthread_mutex_lock");
- while (ibufq_queuelen(globalmsgq) == 0 && !quit)
- pthread_cond_wait(&globalmsgq_cond,
+ error = pthread_mutex_lock(&globalmsgq_mtx);
+ if (error != 0)
+ errx(1, "pthread_mutex_lock: %s",
+ strerror(error));
+ while (ibufq_queuelen(globalmsgq) == 0 && !quit) {
+ error = pthread_cond_wait(&globalmsgq_cond,
&globalmsgq_mtx);
+ if (error != 0)
+ errx(1, "pthread_cond_wait: %s",
+ strerror(error));
+ }
/* enqueue messages to local msgbuf */
msgbuf_concat(myq, globalmsgq);
- if (pthread_mutex_unlock(&globalmsgq_mtx) != 0)
- errx(1, "pthread_mutex_lock");
+ error = pthread_mutex_unlock(&globalmsgq_mtx);
+ if (error != 0)
+ errx(1, "pthread_mutex_lock: %s",
+ strerror(error));
}
if (msgbuf_queuelen(myq) > 0) {
@@ -1190,7 +1212,7 @@ proc_parser(int fd, int nthreads)
struct entity *entp;
struct ibuf *b;
pthread_t writer, *workers;
- int i;
+ int error, i;
/* Only allow access to the cache directory. */
if (unveil(".", "r") == -1)
@@ -1213,11 +1235,12 @@ proc_parser(int fd, int nthreads)
if ((workers = calloc(nthreads, sizeof(*workers))) == NULL)
err(1, NULL);
- if (pthread_create(&writer, NULL, &parse_writer, &fd) != 0)
- errx(1, "pthread_create");
+ if ((error = pthread_create(&writer, NULL, &parse_writer, &fd)) != 0)
+ errx(1, "pthread_create: %s", strerror(error));
for (i = 0; i < nthreads; i++) {
- if (pthread_create(&workers[i], NULL, &parse_worker, NULL) != 0)
- errx(1, "pthread_create");
+ error = pthread_create(&workers[i], NULL, &parse_worker, NULL);
+ if (error != 0)
+ errx(1, "pthread_create: %s", strerror(error));
}
pfd.fd = fd;
@@ -1262,37 +1285,44 @@ proc_parser(int fd, int nthreads)
TAILQ_INSERT_TAIL(&myq, entp, entries);
}
if (!TAILQ_EMPTY(&myq)) {
- if (pthread_mutex_lock(&globalq_mtx) != 0)
- errx(1, "pthread_mutex_lock");
+ error = pthread_mutex_lock(&globalq_mtx);
+ if (error != 0)
+ errx(1, "pthread_mutex_lock: %s",
+ strerror(error));
TAILQ_CONCAT(&globalq, &myq, entries);
- if (pthread_mutex_unlock(&globalq_mtx) != 0)
- errx(1, "pthread_mutex_unlock");
- if (pthread_cond_signal(&globalq_cond) != 0)
- errx(1, "pthread_cond_signal");
+ error = pthread_mutex_unlock(&globalq_mtx);
+ if (error != 0)
+ errx(1, "pthread_mutex_unlock: %s",
+ strerror(error));
+ error = pthread_cond_signal(&globalq_cond);
+ if (error != 0)
+ errx(1, "pthread_cond_signal: %s",
+ strerror(error));
}
}
}
/* signal all threads */
- if (pthread_cond_broadcast(&globalq_cond) != 0)
- errx(1, "pthread_cond_broadcast");
- if (pthread_cond_broadcast(&globalmsgq_cond) != 0)
- errx(1, "pthread_cond_broadcast");
+ if ((error = pthread_cond_broadcast(&globalq_cond)) != 0)
+ errx(1, "pthread_cond_broadcast: %s", strerror(error));
+ if ((error = pthread_cond_broadcast(&globalmsgq_cond)) != 0)
+ errx(1, "pthread_cond_broadcast: %s", strerror(error));
- if (pthread_mutex_lock(&globalq_mtx) != 0)
- errx(1, "pthread_mutex_lock");
+ if ((error = pthread_mutex_lock(&globalq_mtx)) != 0)
+ errx(1, "pthread_mutex_lock: %s", strerror(error));
while ((entp = TAILQ_FIRST(&globalq)) != NULL) {
TAILQ_REMOVE(&globalq, entp, entries);
entity_free(entp);
}
- if (pthread_mutex_unlock(&globalq_mtx) != 0)
- errx(1, "pthread_mutex_unlock");
+ if ((error = pthread_mutex_unlock(&globalq_mtx)) != 0)
+ errx(1, "pthread_mutex_unlock: %s", strerror(error));
- if (pthread_join(writer, NULL) != 0)
- errx(1, "pthread_join writer");
+ if ((error = pthread_join(writer, NULL)) != 0)
+ errx(1, "pthread_join writer: %s", strerror(error));
for (i = 0; i < nthreads; i++) {
- if (pthread_join(workers[i], NULL) != 0)
- errx(1, "pthread_join worker %d", i);
+ if ((error = pthread_join(workers[i], NULL)) != 0)
+ errx(1, "pthread_join worker %d: %s",
+ i, strerror(error));
}
free(workers); /* karl marx */
rpki-client: add error checks for pthread_* calls