Make pr_crit() non-returnable

Code was trying to recover from critical/programming errors, but
most of the time they were being handled the same as validation
errors. This yielded often nonsensical results.

Critical errors now terminate the program. This is obviously
undesired, but is probably safer than doing something close to
undefined.

diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c
index 4047bb12fe9fb968ce78d2692d397fb285318d39..d685e0f47f6019671916915e6610595fb06892f0 100644 (file)
--- a/src/asn1/signed_data.c
+++ b/src/asn1/signed_data.c
@@ -188,7 +188,7 @@ validate_signed_attrs(struct SignerInfo *sinfo, EncapsulatedContentInfo_t *eci)
                            attr->attrValues.list.count);
                }
                if (attrs->list.array == NULL || attrs->list.array[0] == NULL)
-                       return pr_crit("Array size is 1 but array is NULL.");
+                       pr_crit("Array size is 1 but array is NULL.");
 
                error = oid2arcs(&attr->attrType, &attrType);
                if (error)

diff --git a/src/cert_stack.c b/src/cert_stack.c
index ace70c0f7a53439154cc9889c6bca3430f7f4262..bd928a9371763e3efaecb0a7457a4cc0e6efe8cd 100644 (file)
--- a/src/cert_stack.c
+++ b/src/cert_stack.c
@@ -209,7 +209,7 @@ deferstack_push(struct cert_stack *stack, struct deferred_cert *deferred)
        return 0;
 }
 
-static int
+static void
 x509stack_pop(struct cert_stack *stack)
 {
        X509 *cert;
@@ -217,32 +217,30 @@ x509stack_pop(struct cert_stack *stack)
 
        cert = sk_X509_pop(stack->x509s);
        if (cert == NULL)
-               return pr_crit("Attempted to pop empty X509 stack");
+               pr_crit("Attempted to pop empty X509 stack");
        X509_free(cert);
 
        meta = SLIST_FIRST(&stack->metas);
        if (meta == NULL)
-               return pr_crit("Attempted to pop empty metadata stack");
+               pr_crit("Attempted to pop empty metadata stack");
        SLIST_REMOVE_HEAD(&stack->metas, next);
        meta_destroy(meta);
-
-       return 0;
 }
 
+/**
+ * Contract: Returns either 0 or -ENOENT. No other outcomes.
+ */
 int
 deferstack_pop(struct cert_stack *stack, struct deferred_cert *result)
 {
        struct defer_node *node;
-       int error;
 
 again: node = SLIST_FIRST(&stack->defers);
        if (node == NULL)
                return -ENOENT;
 
        if (node->type == DNT_SEPARATOR) {
-               error = x509stack_pop(stack);
-               if (error)
-                       return error;
+               x509stack_pop(stack);
 
                SLIST_REMOVE_HEAD(&stack->defers, next);
                defer_destroy(node);
@@ -346,10 +344,8 @@ x509stack_cancel(struct cert_stack *stack)
        x509stack_pop(stack);
 
        defer_separator = SLIST_FIRST(&stack->defers);
-       if (defer_separator == NULL) {
+       if (defer_separator == NULL)
                pr_crit("Attempted to pop empty defer stack");
-               return;
-       }
        SLIST_REMOVE_HEAD(&stack->defers, next);
        defer_destroy(defer_separator);
 }
@@ -383,7 +379,7 @@ get_current_file_name(char **_result)
 
        tmp = fnstack_peek();
        if (tmp == NULL)
-               return pr_crit("The file name stack is empty.");
+               pr_crit("The file name stack is empty.");
 
        result = strdup(tmp);
        if (result == NULL)

diff --git a/src/certificate_refs.c b/src/certificate_refs.c
index 2a4fca5717b2f5b49f498d15b095655bd00de933..ed8e7826058a12fd700450743849dec34b4fcb22 100644 (file)
--- a/src/certificate_refs.c
+++ b/src/certificate_refs.c
@@ -26,11 +26,11 @@ validate_cdp(struct certificate_refs *refs, struct rpp const *pp)
        struct rpki_uri *pp_crl;
 
        if (refs->crldp == NULL)
-               return pr_crit("Certificate's CRL Distribution Point was not recorded.");
+               pr_crit("Certificate's CRL Distribution Point was not recorded.");
 
        pp_crl = rpp_get_crl(pp);
        if (pp_crl == NULL)
-               return pr_crit("Manifest's CRL was not recorded.");
+               pr_crit("Manifest's CRL was not recorded.");
 
        if (strcmp(refs->crldp, uri_get_global(pp_crl)) != 0) {
                return pr_err("Certificate's CRL Distribution Point ('%s') does not match manifest's CRL ('%s').",
@@ -47,14 +47,14 @@ validate_aia(struct certificate_refs *refs)
        struct rpki_uri *parent;
 
        if (refs->caIssuers == NULL)
-               return pr_crit("Certificate's AIA was not recorded.");
+               pr_crit("Certificate's AIA was not recorded.");
 
        state = state_retrieve();
        if (state == NULL)
                return -EINVAL;
        parent = x509stack_peek_uri(validation_certstack(state));
        if (parent == NULL)
-               return pr_crit("CA certificate has no parent.");
+               pr_crit("CA certificate has no parent.");
 
        if (!uri_equals(refs->caIssuers, parent)) {
                return pr_err("Certificate's AIA ('%s') does not match parent's URI ('%s').",
@@ -70,7 +70,7 @@ validate_signedObject(struct certificate_refs *refs,
     struct rpki_uri *signedObject_uri)
 {
        if (refs->signedObject == NULL)
-               return pr_crit("Certificate's signedObject was not recorded.");
+               pr_crit("Certificate's signedObject was not recorded.");
 
        if (!uri_equals(refs->signedObject, signedObject_uri)) {
                return pr_err("Certificate's signedObject ('%s') does not match the URI of its own signed object (%s).",
@@ -104,10 +104,9 @@ refs_validate_ca(struct certificate_refs *refs, struct rpp const *pp)
        if (error)
                return error;
 
-       if (refs->signedObject != NULL) {
-               return pr_crit("CA summary has a signedObject ('%s').",
+       if (refs->signedObject != NULL)
+               pr_crit("CA summary has a signedObject ('%s').",
                    uri_get_printable(refs->signedObject));
-       }
 
        return 0;
 }

diff --git a/src/common.c b/src/common.c
index c05f89d2a0dd662cc164b3ed26f6bd5eb75d6509..19f50226cf9221a20f7c20e76b48515757ea3bbb 100644 (file)
--- a/src/common.c
+++ b/src/common.c
@@ -68,24 +68,20 @@ rwlock_unlock(pthread_rwlock_t *lock)
        }
 }
 
-int
+void
 close_thread(pthread_t thread, char const *what)
 {
        int error;
 
        error = pthread_cancel(thread);
-       if (error && error != ESRCH) {
+       if (error && error != ESRCH)
                pr_crit("pthread_cancel() threw %d on the '%s' thread.",
                    error, what);
-               return error;
-       }
 
        error = pthread_join(thread, NULL);
        if (error)
                pr_crit("pthread_join() threw %d on the '%s' thread.",
                    error, what);
-
-       return error;
 }
 
 static int

diff --git a/src/common.h b/src/common.h
index 57851697b294c56de6310ea0758d0d44818e0a92..c636968c98cc23239ba79a14851997d6992cd178 100644 (file)
--- a/src/common.h
+++ b/src/common.h
@@ -35,7 +35,7 @@ void rwlock_write_lock(pthread_rwlock_t *);
 void rwlock_unlock(pthread_rwlock_t *);
 
 /** Also boilerplate. */
-int close_thread(pthread_t thread, char const *what);
+void close_thread(pthread_t thread, char const *what);
 
 typedef int (*process_file_cb)(char const *, void *);
 int process_file_or_dir(char const *, char const *, process_file_cb, void *);

diff --git a/src/config.c b/src/config.c
index dfc807fb219221ff325056e893d1ba49108a6205..eb0d57735e04c00e54ee185bfe1be41e5da8cf23 100644 (file)
--- a/src/config.c
+++ b/src/config.c
@@ -699,11 +699,6 @@ config_get_rsync_args(bool is_ta)
        }
 
        pr_crit("Invalid sync strategy: '%u'", rpki_config.sync_strategy);
-       /*
-        * Return something usable anyway; don't want to check NULL.
-        * This is supposed to be unreachable code anyway.
-        */
-       return &rpki_config.rsync.args.recursive;
 }
 
 void

diff --git a/src/line_file.c b/src/line_file.c
index ec3d154b20abef280e4ee196330ce2096d9f717b..a0f6b68a6d2783f07ab3a6f88307da49cd874b37 100644 (file)
--- a/src/line_file.c
+++ b/src/line_file.c
@@ -107,7 +107,7 @@ lfile_read(struct line_file *lfile, char **result)
                        return pr_errno(error, "Error while reading file");
                if (feof(lfile->file))
                        return 0;
-               return pr_crit("Supposedly unreachable code reached. ferror:%d feof:%d",
+               pr_crit("Supposedly unreachable code reached. ferror:%d feof:%d",
                    ferror(lfile->file), feof(lfile->file));
        }
 

diff --git a/src/log.c b/src/log.c
index e2ce9491cba0bb445049099657a569310a18bfd4..e343de997cb4885746c73bd4f499354a2b48c2a1 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -253,7 +253,7 @@ pr_enomem(void)
        return -ENOMEM;
 }
 
-int
+__dead void
 pr_crit(const char *format, ...)
 {
        va_list args;
@@ -269,7 +269,7 @@ pr_crit(const char *format, ...)
        PR_SUFFIX(STDERR);
 
        print_stack_trace();
-       return -EINVAL;
+       exit(-1);
 }
 
 /**
@@ -296,5 +296,5 @@ incidence(enum incidence_id id, const char *format, ...)
                return -EINVAL;
        }
 
-       return pr_crit("Unknown incidence action: %u", action);
+       pr_crit("Unknown incidence action: %u", action);
 }

diff --git a/src/log.h b/src/log.h
index f7f2e3a664a763fd371cb366b8da63d84d77cf7d..a16f6206ef56d53ed38bf78d5002c2d64ce20c91 100644 (file)
--- a/src/log.h
+++ b/src/log.h
@@ -1,8 +1,21 @@
 #ifndef SRC_LOG_H_
 #define SRC_LOG_H_
 
+#include <sys/cdefs.h>
 #include "incidence/incidence.h"
 
+/*
+ * __dead is supposed to be defined in sys/cdefs.h, but is apparently not
+ * portable.
+ */
+#ifndef __dead
+#if __GNUC__
+#define __dead __attribute__ ((noreturn))
+#else
+#define __dead
+#endif
+#endif
+
 /*
  * I know that the OpenBSD style guide says that we shouldn't declare our own
  * error printing functions, but we kind of need to do it:
@@ -59,7 +72,7 @@ int pr_err(const char *, ...) CHECK_FORMAT(1, 2);
 int pr_errno(int, const char *, ...) CHECK_FORMAT(2, 3);
 int crypto_err(const char *, ...) CHECK_FORMAT(1, 2);
 int pr_enomem(void);
-int pr_crit(const char *, ...) CHECK_FORMAT(1, 2);
+__dead void pr_crit(const char *, ...) CHECK_FORMAT(1, 2);
 
 int incidence(enum incidence_id, const char *, ...) CHECK_FORMAT(2, 3);
 

diff --git a/src/object/certificate.c b/src/object/certificate.c
index 74318b92e8feae01d2309b9b5f6d3a310fb8e193..21412fd7ad7e0ea6435f9dbf653595d51cce0e5a 100644 (file)
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -162,7 +162,7 @@ validate_spki(X509_PUBKEY *cert_spki)
 
        tal = validation_tal(state);
        if (tal == NULL)
-               return pr_crit("Validation state has no TAL.");
+               pr_crit("Validation state has no TAL.");
 
        /*
         * We have a problem at this point:
@@ -316,7 +316,7 @@ struct progress {
 /**
  * Skip the "T" part of a TLV.
  */
-static int
+static void
 skip_t(ANY_t *content, struct progress *p, unsigned int tag)
 {
        /*
@@ -324,67 +324,57 @@ skip_t(ANY_t *content, struct progress *p, unsigned int tag)
         * to be validated by this point.
         */
 
-       if (content->buf[p->offset] != tag) {
-               return pr_crit("Expected tag 0x%x, got 0x%x", tag,
+       if (content->buf[p->offset] != tag)
+               pr_crit("Expected tag 0x%x, got 0x%x", tag,
                    content->buf[p->offset]);
-       }
 
        if (p->remaining == 0)
-               return pr_crit("Buffer seems to be truncated");
+               pr_crit("Buffer seems to be truncated");
        p->offset++;
        p->remaining--;
-       return 0;
 }
 
 /**
  * Skip the "TL" part of a TLV.
  */
-static int
+static void
 skip_tl(ANY_t *content, struct progress *p, unsigned int tag)
 {
        ssize_t len_len; /* Length of the length field */
        ber_tlv_len_t value_len; /* Length of the value */
-       int error;
 
-       error = skip_t(content, p, tag);
-       if (error)
-               return error;
+       skip_t(content, p, tag);
 
        len_len = ber_fetch_length(true, &content->buf[p->offset], p->remaining,
            &value_len);
        if (len_len == -1)
-               return pr_crit("Could not decipher length (Cause is unknown)");
+               pr_crit("Could not decipher length (Cause is unknown)");
        if (len_len == 0)
-               return pr_crit("Buffer seems to be truncated");
+               pr_crit("Buffer seems to be truncated");
 
        p->offset += len_len;
        p->remaining -= len_len;
-       return 0;
 }
 
-static int
+static void
 skip_tlv(ANY_t *content, struct progress *p, unsigned int tag)
 {
        int is_constructed;
        int skip;
-       int error;
 
        is_constructed = BER_TLV_CONSTRUCTED(&content->buf[p->offset]);
 
-       error = skip_t(content, p, tag);
-       if (error)
-               return error;
+       skip_t(content, p, tag);
 
        skip = ber_skip_length(NULL, is_constructed, &content->buf[p->offset],
            p->remaining);
        if (skip == -1)
-               return pr_crit("Could not skip length (Cause is unknown)");
+               pr_crit("Could not skip length (Cause is unknown)");
        if (skip == 0)
-               return pr_crit("Buffer seems to be truncated");
+               pr_crit("Buffer seems to be truncated");
 
        p->offset += skip;
        p->remaining -= skip;
-       return 0;
 }
 
 /**
@@ -395,7 +385,7 @@ struct encoded_signedAttrs {
        ber_tlv_len_t size;
 };
 
-static int
+static void
 find_signedAttrs(ANY_t *signedData, struct encoded_signedAttrs *result)
 {
 #define INTEGER_TAG            0x02
@@ -403,7 +393,6 @@ find_signedAttrs(ANY_t *signedData, struct encoded_signedAttrs *result)
 #define SET_TAG                        0x31
 
        struct progress p;
-       int error;
        ssize_t len_len;
 
        /* Reference: rfc5652-12.1.asn1 */
@@ -412,67 +401,43 @@ find_signedAttrs(ANY_t *signedData, struct encoded_signedAttrs *result)
        p.remaining = signedData->size;
 
        /* SignedData: SEQUENCE */
-       error = skip_tl(signedData, &p, SEQUENCE_TAG);
-       if (error)
-               return error;
+       skip_tl(signedData, &p, SEQUENCE_TAG);
 
        /* SignedData.version: CMSVersion -> INTEGER */
-       error = skip_tlv(signedData, &p, INTEGER_TAG);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, INTEGER_TAG);
        /* SignedData.digestAlgorithms: DigestAlgorithmIdentifiers -> SET */
-       error = skip_tlv(signedData, &p, SET_TAG);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, SET_TAG);
        /* SignedData.encapContentInfo: EncapsulatedContentInfo -> SEQUENCE */
-       error = skip_tlv(signedData, &p, SEQUENCE_TAG);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, SEQUENCE_TAG);
        /* SignedData.certificates: CertificateSet -> SET */
-       error = skip_tlv(signedData, &p, 0xA0);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, 0xA0);
        /* SignedData.signerInfos: SignerInfos -> SET OF SEQUENCE */
-       error = skip_tl(signedData, &p, SET_TAG);
-       if (error)
-               return error;
-       error = skip_tl(signedData, &p, SEQUENCE_TAG);
-       if (error)
-               return error;
+       skip_tl(signedData, &p, SET_TAG);
+       skip_tl(signedData, &p, SEQUENCE_TAG);
 
        /* SignedData.signerInfos.version: CMSVersion -> INTEGER */
-       error = skip_tlv(signedData, &p, INTEGER_TAG);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, INTEGER_TAG);
        /*
         * SignedData.signerInfos.sid: SignerIdentifier -> CHOICE -> always
         * subjectKeyIdentifier, which is a [0].
         */
-       error = skip_tlv(signedData, &p, 0x80);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, 0x80);
        /* SignedData.signerInfos.digestAlgorithm: DigestAlgorithmIdentifier
         * -> AlgorithmIdentifier -> SEQUENCE */
-       error = skip_tlv(signedData, &p, SEQUENCE_TAG);
-       if (error)
-               return error;
+       skip_tlv(signedData, &p, SEQUENCE_TAG);
 
        /* SignedData.signerInfos.signedAttrs: SignedAttributes -> SET */
        /* We will need to replace the tag 0xA0 with 0x31, so skip it as well */
-       error = skip_t(signedData, &p, 0xA0);
-       if (error)
-               return error;
+       skip_t(signedData, &p, 0xA0);
 
        result->buffer = &signedData->buf[p.offset];
        len_len = ber_fetch_length(true, result->buffer,
            p.remaining, &result->size);
        if (len_len == -1)
-               return pr_crit("Could not decipher length (Cause is unknown)");
+               pr_crit("Could not decipher length (Cause is unknown)");
        if (len_len == 0)
-               return pr_crit("Buffer seems to be truncated");
+               pr_crit("Buffer seems to be truncated");
        result->size += len_len;
-
-       return 0;
 }
 
 /*
@@ -554,9 +519,7 @@ certificate_validate_signature(X509 *cert, ANY_t *signedData,
         * Second option it is.
         */
 
-       error = find_signedAttrs(signedData, &signedAttrs);
-       if (error)
-               goto end;
+       find_signedAttrs(signedData, &signedAttrs);
 
        error = EVP_DigestVerifyUpdate(ctx, &EXPLICIT_SET_OF_TAG,
            sizeof(EXPLICIT_SET_OF_TAG));
@@ -834,7 +797,7 @@ certificate_get_resources(X509 *cert, struct resources *resources)
                    "8360", "6484");
        }
 
-       return pr_crit("Unknown policy: %u", policy);
+       pr_crit("Unknown policy: %u", policy);
 }
 
 static bool

diff --git a/src/object/roa.c b/src/object/roa.c
index 4e563fab158b024feff56b8ff5925a375741de12..cfc8d0a6bed1b7de2c7890b14f021f2032ffccda 100644 (file)
--- a/src/object/roa.c
+++ b/src/object/roa.c
@@ -152,7 +152,7 @@ __handle_roa(struct RouteOriginAttestation *roa, struct resources *parent)
        /* rfc6482#section-3.3 */
 
        if (roa->ipAddrBlocks.list.array == NULL)
-               return pr_crit("ipAddrBlocks array is NULL.");
+               pr_crit("ipAddrBlocks array is NULL.");
 
        for (b = 0; b < roa->ipAddrBlocks.list.count; b++) {
                block = roa->ipAddrBlocks.list.array[b];

diff --git a/src/object/tal.c b/src/object/tal.c
index 29f47135e36ae7437d06d9f514b6f5854ef4ace2..7ff8c0eaa5b393c9a5a83c2acaab4b7d203afc8b 100644 (file)
--- a/src/object/tal.c
+++ b/src/object/tal.c
@@ -328,24 +328,20 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg)
                        goto fail; /* Reject the TAL. */
                }
 
-               error = pr_crit("Unknown public key state: %u",
+               pr_crit("Unknown public key state: %u",
                    validation_pubkey_state(state));
-               goto fail;
        }
 
        /*
         * From now on, the tree should be considered valid, even if subsequent
         * certificates fail.
         * (the root validated successfully; subtrees are isolated problems.)
-        * Only critical errors should trigger negative result codes.
         */
 
        /* Handle every other certificate. */
        certstack = validation_certstack(state);
-       if (certstack == NULL) {
-               error = pr_crit("Validation state has no certificate stack");
-               goto fail;
-       }
+       if (certstack == NULL)
+               pr_crit("Validation state has no certificate stack");
 
        do {
                error = deferstack_pop(certstack, &deferred);
@@ -353,9 +349,8 @@ handle_tal_uri(struct tal *tal, struct rpki_uri *uri, void *arg)
                        /* No more certificates left; we're done. */
                        error = 1;
                        goto end;
-               }
-               if (error) /* All other errors are critical, currently */
-                       goto fail;
+               } else if (error) /* All other errors are critical, currently */
+                       pr_crit("deferstack_pop() returned illegal %d.", error);
 
                /*
                 * Ignore result code; remaining certificates are unrelated,

diff --git a/src/resource.c b/src/resource.c
index 36e1f7f84fa04c217673d8a5383ec617702b9d69..fcbe7b96001b8151f05a9fd5cf199fa36430d529 100644 (file)
--- a/src/resource.c
+++ b/src/resource.c
@@ -100,7 +100,7 @@ inherit_aors(struct resources *resources, int family)
 
        parent = get_parent_resources();
        if (parent == NULL)
-               return pr_crit("Parent has no resources.");
+               pr_crit("Parent has no resources.");
 
        switch (family) {
        case AF_INET:
@@ -122,7 +122,7 @@ inherit_aors(struct resources *resources, int family)
                return 0;
        }
 
-       return pr_crit("Unknown address family '%d'", family);
+       pr_crit("Unknown address family '%d'", family);
 }
 
 static int
@@ -225,7 +225,7 @@ add_prefix(struct resources *resources, int family, IPAddress_t *addr)
                return add_prefix6(resources, addr);
        }
 
-       return pr_crit("Unknown address family '%d'", family);
+       pr_crit("Unknown address family '%d'", family);
 }
 
 static int
@@ -330,7 +330,7 @@ add_range(struct resources *resources, int family, IPAddressRange_t *range)
                return add_range6(resources, range);
        }
 
-       return pr_crit("Unknown address family '%d'", family);
+       pr_crit("Unknown address family '%d'", family);
 }
 
 static int
@@ -402,7 +402,7 @@ inherit_asiors(struct resources *resources)
 
        parent = get_parent_resources();
        if (parent == NULL)
-               return pr_crit("Parent has no resources.");
+               pr_crit("Parent has no resources.");
 
        if (resources->asns != NULL)
                return pr_err("Certificate inherits ASN resources while also defining others of its own.");

diff --git a/src/rsync/rsync.c b/src/rsync/rsync.c
index d023f683d5e5490efb75896a87a2ddbe44f398ab..fc1afe6bafa822eb8b67b827273fdbe345fb2943 100644 (file)
--- a/src/rsync/rsync.c
+++ b/src/rsync/rsync.c
@@ -150,10 +150,10 @@ get_rsync_uri(struct rpki_uri *requested_uri, bool is_ta,
        case SYNC_STRICT:
                return handle_strict_strategy(requested_uri, rsync_uri);
        case SYNC_OFF:
-               return pr_crit("Supposedly unreachable code reached.");
+               break;
        }
 
-       return pr_crit("Unknown sync strategy: %u", config_get_sync_strategy());
+       pr_crit("Invalid sync strategy: %u", config_get_sync_strategy());
 }
 
 static int

diff --git a/src/rtr/db/delta.c b/src/rtr/db/delta.c
index 4b416a14a374402241e1b9e792cb6f1f1a252ddd..691bdc695fc00b7b371ddd57fcba26414f6fbe2d 100644 (file)
--- a/src/rtr/db/delta.c
+++ b/src/rtr/db/delta.c
@@ -91,7 +91,7 @@ deltas_add_roa_v4(struct deltas *deltas, uint32_t as, struct v4_address *addr,
                return deltas_v4_add(&deltas->v4.removes, &delta);
        }
 
-       return pr_crit("Unknown delta operation: %d", op);
+       pr_crit("Unknown delta operation: %d", op);
 }
 
 int
@@ -111,7 +111,7 @@ deltas_add_roa_v6(struct deltas *deltas, uint32_t as, struct v6_address *addr,
                return deltas_v6_add(&deltas->v6.removes, &delta);
        }
 
-       return pr_crit("Unknown delta operation: %d", op);
+       pr_crit("Unknown delta operation: %d", op);
 }
 
 bool

diff --git a/src/rtr/db/roa_table.c b/src/rtr/db/roa_table.c
index 47b83742a7e24b7bbbf34da516b3d4c5096971f0..e69b85ff225e89ba17ad46743b4976cf940f8a70 100644 (file)
--- a/src/rtr/db/roa_table.c
+++ b/src/rtr/db/roa_table.c
@@ -107,7 +107,8 @@ duplicate_roa(struct roa_table *dst, struct hashable_roa *new)
                return rtrhandler_handle_roa_v6(dst, vrp.asn, &prefix6,
                    vrp.max_prefix_length);
        }
-       return pr_crit("Unknown address family: %d", vrp.addr_fam);
+
+       pr_crit("Unknown address family: %d", vrp.addr_fam);
 }
 
 int
@@ -219,7 +220,7 @@ add_delta(struct deltas *deltas, struct hashable_roa *roa, int op)
                return deltas_add_roa_v6(deltas, roa->data.asn, &addr.v6, op);
        }
 
-       return pr_crit("Unknown address family: %d", roa->data.addr_fam);
+       pr_crit("Unknown address family: %d", roa->data.addr_fam);
 }
 
 /*

diff --git a/src/rtr/pdu_sender.c b/src/rtr/pdu_sender.c
index a54872fe57653d540be0eab081ac0de7716800b9..b66686d6ee4f1a1473a570bbe065174c16c9e190 100644 (file)
--- a/src/rtr/pdu_sender.c
+++ b/src/rtr/pdu_sender.c
@@ -69,7 +69,7 @@ send_serial_notify_pdu(int fd, serial_t start_serial)
 
        len = serialize_serial_notify_pdu(&pdu, data);
        if (len != RTRPDU_SERIAL_NOTIFY_LEN)
-               return pr_crit("Serialized Serial Notify is %zu bytes.", len);
+               pr_crit("Serialized Serial Notify is %zu bytes.", len);
 
        return send_response(fd, data, len);
 }
@@ -87,7 +87,7 @@ send_cache_reset_pdu(int fd)
 
        len = serialize_cache_reset_pdu(&pdu, data);
        if (len != RTRPDU_CACHE_RESET_LEN)
-               return pr_crit("Serialized Cache Reset is %zu bytes.", len);
+               pr_crit("Serialized Cache Reset is %zu bytes.", len);
 
        return send_response(fd, data, len);
 }
@@ -106,7 +106,7 @@ send_cache_response_pdu(int fd)
 
        len = serialize_cache_response_pdu(&pdu, data);
        if (len != RTRPDU_CACHE_RESPONSE_LEN)
-               return pr_crit("Serialized Cache Response is %zu bytes.", len);
+               pr_crit("Serialized Cache Response is %zu bytes.", len);
 
        return send_response(fd, data, len);
 }
@@ -130,7 +130,7 @@ send_ipv4_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags)
 
        len = serialize_ipv4_prefix_pdu(&pdu, data);
        if (len != RTRPDU_IPV4_PREFIX_LEN)
-               return pr_crit("Serialized IPv4 Prefix is %zu bytes.", len);
+               pr_crit("Serialized IPv4 Prefix is %zu bytes.", len);
 
        return send_response(fd, data, len);
 }
@@ -154,7 +154,7 @@ send_ipv6_prefix_pdu(int fd, struct vrp const *vrp, uint8_t flags)
 
        len = serialize_ipv6_prefix_pdu(&pdu, data);
        if (len != RTRPDU_IPV6_PREFIX_LEN)
-               return pr_crit("Serialized IPv6 Prefix is %zu bytes.", len);
+               pr_crit("Serialized IPv6 Prefix is %zu bytes.", len);
 
        return send_response(fd, data, len);
 }
@@ -274,7 +274,7 @@ send_end_of_data_pdu(int fd, serial_t end_serial)
 
        len = serialize_end_of_data_pdu(&pdu, data);
        if (len != RTRPDU_END_OF_DATA_LEN)
-               return pr_crit("Serialized End of Data is %zu bytes.", len);
+               pr_crit("Serialized End of Data is %zu bytes.", len);
 
        error = send_response(fd, data, len);
        if (error)
@@ -318,15 +318,12 @@ send_error_report_pdu(int fd, uint16_t code, struct rtr_request const *request,
                return pr_enomem();
 
        len = serialize_error_report_pdu(&pdu, data);
-       if (len != pdu.header.length) {
-               error = pr_crit("Serialized Error Report PDU is %zu bytes, not the expected %u.",
+       if (len != pdu.header.length)
+               pr_crit("Serialized Error Report PDU is %zu bytes, not the expected %u.",
                    len, pdu.header.length);
-               goto end;
-       }
 
        error = send_response(fd, data, len);
 
-end:
        free(data);
        return error;
 }

diff --git a/src/rtr/primitive_reader.c b/src/rtr/primitive_reader.c
index 7c7c7b4a1064e9ad6ef88d74f2461291f4d3daf2..e05b35ad68ba3013a8603b7e857ed6a82a127d9b 100644 (file)
--- a/src/rtr/primitive_reader.c
+++ b/src/rtr/primitive_reader.c
@@ -60,18 +60,17 @@ pdu_reader_init(struct pdu_reader *reader, int fd, unsigned char *buffer,
        return read_exact(fd, reader->buffer, size, allow_eof);
 }
 
-static int
+__dead static void
 insufficient_bytes(void)
 {
        pr_crit("Attempted to read past the end of a PDU Reader.");
-       return -EPIPE;
 }
 
 int
 read_int8(struct pdu_reader *reader, uint8_t *result)
 {
        if (reader->size < 1)
-               return insufficient_bytes();
+               insufficient_bytes();
 
        *result = reader->buffer[0];
        reader->buffer++;
@@ -84,7 +83,7 @@ int
 read_int16(struct pdu_reader *reader, uint16_t *result)
 {
        if (reader->size < 2)
-               return insufficient_bytes();
+               insufficient_bytes();
 
        *result = (((uint16_t)reader->buffer[0]) << 8)
                | (((uint16_t)reader->buffer[1])     );
@@ -98,7 +97,7 @@ int
 read_int32(struct pdu_reader *reader, uint32_t *result)
 {
        if (reader->size < 4)
-               return insufficient_bytes();
+               insufficient_bytes();
 
        *result = (((uint32_t)reader->buffer[0]) << 24)
                | (((uint32_t)reader->buffer[1]) << 16)
@@ -243,7 +242,7 @@ int
 read_bytes(struct pdu_reader *reader, unsigned char *result, size_t num)
 {
        if (reader->size < num)
-               return insufficient_bytes();
+               insufficient_bytes();
 
        memcpy(result, reader->buffer, num);
        reader->buffer += num;

diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c
index 8e19c1c116326584b36db8addc0398c49940b36f..e0d86b95df175132740aa6e0d62931937ac761dc 100644 (file)
--- a/src/rtr/rtr.c
+++ b/src/rtr/rtr.c
@@ -345,7 +345,8 @@ end_clients(void)
 static int
 join_thread(pthread_t tid, void *arg)
 {
-       return close_thread(tid, "Client");
+       close_thread(tid, "Client");
+       return 0;
 }
 
 /*

diff --git a/src/slurm/slurm_loader.c b/src/slurm/slurm_loader.c
index 6444e685c864d017ce5e766bb7554071bc7c4e93..9b504d6f71b200d5269b19c6ce42b6a48bdffbba 100644 (file)
--- a/src/slurm/slurm_loader.c
+++ b/src/slurm/slurm_loader.c
@@ -71,7 +71,8 @@ slurm_pfx_assertions_add(struct slurm_prefix *prefix, void *arg)
                return rtrhandler_handle_roa_v6(table, vrp.asn, &prefix6,
                    vrp.max_prefix_length);
        }
-       return -pr_crit("Unkown addr family type");
+
+       pr_crit("Unkown addr family type: %u", vrp.addr_fam);
 }
 
 static int

diff --git a/src/sorted_array.c b/src/sorted_array.c
index 42434382777736c618d94d46edd6a9a3b38cff7f..2200331b366ed610bebb47a42acd5e53cdde42f8 100644 (file)
--- a/src/sorted_array.c
+++ b/src/sorted_array.c
@@ -105,7 +105,7 @@ compare(struct sorted_array *sarray, void *new)
                return -EINTERSECTION;
        }
 
-       return pr_crit("Unknown comparison value: %u", cmp);
+       pr_crit("Unknown comparison value: %u", cmp);
 }
 
 int
@@ -174,7 +174,6 @@ sarray_contains(struct sorted_array *sarray, void *elem)
                }
 
                pr_crit("Unknown comparison value: %u", cmp);
-               return false;
        }
 
        return false;

diff --git a/src/updates_daemon.c b/src/updates_daemon.c
index b7713d50d08078fbda81ba2c2797a87e795e0402..7868ff27cb0ff6f01783e88dc03021fcb7947621 100644 (file)
--- a/src/updates_daemon.c
+++ b/src/updates_daemon.c
@@ -57,6 +57,5 @@ updates_daemon_start(void)
 void
 updates_daemon_destroy(void)
 {
-       /* Not much to do with the error code. */
        close_thread(thread, "Validation");
 }

diff --git a/src/uri.c b/src/uri.c
index 5f231b91cf7c633fd20e2c9b70f9a3ec56017e2e..9a5b1f5d91e03d7a388bf4981beb7cbfa9c74add 100644 (file)
--- a/src/uri.c
+++ b/src/uri.c
@@ -434,5 +434,4 @@ uri_get_printable(struct rpki_uri *uri)
        }
 
        pr_crit("Unknown file name format: %u", format);
-       return uri->global;
 }

diff --git a/src/validation_handler.c b/src/validation_handler.c
index 83348da4c57c783cbcbd1f8df2b54fc07666d840..266658c8259f56412fce85a9a2576ee9d78ef6d8 100644 (file)
--- a/src/validation_handler.c
+++ b/src/validation_handler.c
@@ -15,7 +15,7 @@ get_current_threads_handler(struct validation_handler const **result)
                return -EINVAL;
        handler = validation_get_validation_handler(state);
        if (handler == NULL)
-               return pr_crit("This thread lacks a validation handler.");
+               pr_crit("This thread lacks a validation handler.");
 
        *result = handler;
        return 0;