From: Alberto Leiva Popper Date: Tue, 4 Jun 2019 16:09:47 +0000 (-0500) Subject: Make pr_crit() non-returnable X-Git-Tag: v0.0.2~12^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f851bd9e17b42d450b315689392f665a2f417fd9;p=thirdparty%2FFORT-validator.git 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 4047bb12..d685e0f4 100644 --- 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 ace70c0f..bd928a93 100644 --- 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 2a4fca57..ed8e7826 100644 --- 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 c05f89d2..19f50226 100644 --- 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 57851697..c636968c 100644 --- 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 dfc807fb..eb0d5773 100644 --- 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 ec3d154b..a0f6b68a 100644 --- 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 e2ce9491..e343de99 100644 --- 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 f7f2e3a6..a16f6206 100644 --- a/src/log.h +++ b/src/log.h @@ -1,8 +1,21 @@ #ifndef SRC_LOG_H_ #define SRC_LOG_H_ +#include #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 74318b92..21412fd7 100644 --- 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 4e563fab..cfc8d0a6 100644 --- 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 29f47135..7ff8c0ea 100644 --- 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 36e1f7f8..fcbe7b96 100644 --- 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 d023f683..fc1afe6b 100644 --- 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 4b416a14..691bdc69 100644 --- 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 47b83742..e69b85ff 100644 --- 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 a54872fe..b66686d6 100644 --- 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 7c7c7b4a..e05b35ad 100644 --- 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 8e19c1c1..e0d86b95 100644 --- 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 6444e685..9b504d6f 100644 --- 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 42434382..2200331b 100644 --- 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 b7713d50..7868ff27 100644 --- 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 5f231b91..9a5b1f5d 100644 --- 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 83348da4..266658c8 100644 --- 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;