Add a bunch of GCC warning flags

Found lots of bugs because of them. Most are fixed.

diff --git a/src/Makefile.am b/src/Makefile.am
index 5379704e651f646f041e27465c710c4167abeceb..c9637466e0cf3e7d258e4d49e424677ec381af86 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,4 +1,5 @@
 # Comment these out during releases.
+# Also increase optimization I guess (-O0 -> -On)
 CFLAGS_DEBUG  = -DDEBUG
 LDFLAGS_DEBUG = -rdynamic
 
@@ -47,5 +48,79 @@ rpki_validator_SOURCES += resource/ip4.h resource/ip4.c
 rpki_validator_SOURCES += resource/ip6.h resource/ip6.c
 rpki_validator_SOURCES += resource/asn.h resource/asn.c
 
-rpki_validator_CFLAGS  = -pedantic -Wall -std=gnu11 -O0 -g $(CFLAGS_DEBUG)
+rpki_validator_CFLAGS  = -Wall
+# Feel free to temporarily remove this one if you're not using gcc 7.3.0.
+rpki_validator_CFLAGS += $(GCC_WARNS)
+rpki_validator_CFLAGS += -std=gnu11 -O0 -g $(CFLAGS_DEBUG)
 rpki_validator_LDFLAGS = $(LDFLAGS_DEBUG)
+
+# I'm tired of scrolling up, but feel free to comment this out.
+GCC_WARNS  = -fmax-errors=1
+
+# Please ready some arguments if you want to permanently remove some of these
+# flags. I gave a quick read to the documentation of all of these warnings, and
+# generally liked each of them.
+GCC_WARNS += -Wpedantic -pedantic-errors -Waddress -Walloc-zero -Walloca
+GCC_WARNS += -Wno-aggressive-loop-optimizations -Warray-bounds=2 -Wbool-compare
+GCC_WARNS += -Wbool-operation -Wno-builtin-declaration-mismatch -Wcast-align
+GCC_WARNS += -Wcast-qual -Wchar-subscripts -Wchkp -Wclobbered -Wcomment
+GCC_WARNS += -Wdangling-else -Wdate-time -Wdisabled-optimization
+GCC_WARNS += -Wdouble-promotion -Wduplicated-branches -Wduplicated-cond
+GCC_WARNS += -Wempty-body -Wenum-compare -Wexpansion-to-defined -Wfloat-equal
+GCC_WARNS += -Wformat -Wformat-nonliteral -Wformat-overflow=2 -Wformat-security
+GCC_WARNS += -Wformat-signedness -Wformat-truncation=2 -Wformat-y2k
+GCC_WARNS += -Wframe-address -Wjump-misses-init -Wignored-qualifiers
+GCC_WARNS += -Wignored-attributes -Wincompatible-pointer-types
+
+# This is a fun one. Write "/* fallthrough */" to prevent a warning whenever
+# switch cases do not break.
+GCC_WARNS += -Wimplicit-fallthrough
+
+GCC_WARNS += -Wimplicit-function-declaration -Wimplicit-int -Winit-self -Winline
+GCC_WARNS += -Wint-in-bool-context -Winvalid-memory-model -Winvalid-pch
+GCC_WARNS += -Wlogical-op -Wlogical-not-parentheses -Wlong-long -Wmain
+GCC_WARNS += -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args
+GCC_WARNS += -Wmisleading-indentation -Wmissing-braces -Wmissing-include-dirs
+GCC_WARNS += -Wnonnull -Wnonnull-compare -Wnormalized -Wnull-dereference
+
+# This one seems to be undocumented.
+GCC_WARNS += -Wodr
+
+# "Warn if the vectorizer cost model overrides the OpenMP or the Cilk Plus simd
+# directive set by user."
+# ... What?
+GCC_WARNS += -Wopenmp-simd
+
+GCC_WARNS += -Woverride-init-side-effects -Woverlength-strings -Wpacked
+GCC_WARNS += -Wpacked-bitfield-compat -Wparentheses -Wpointer-arith
+GCC_WARNS += -Wpointer-compare -Wredundant-decls -Wrestrict -Wreturn-type
+GCC_WARNS += -Wsequence-point -Wshadow -Wshift-overflow=2 -Wshift-count-negative
+GCC_WARNS += -Wshift-count-overflow -Wshift-negative-value -Wfloat-conversion
+GCC_WARNS += -Wsizeof-pointer-memaccess -Wsizeof-array-argument
+GCC_WARNS += -Wstack-protector -Wstrict-aliasing -Wstrict-overflow=5
+GCC_WARNS += -Wstringop-overflow=4 -Wsuggest-final-types -Wsuggest-final-methods
+GCC_WARNS += -Wmissing-format-attribute -Wswitch -Wswitch-bool -Wswitch-enum
+GCC_WARNS += -Wswitch-unreachable -Wsync-nand -Wtautological-compare
+GCC_WARNS += -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef -Wuninitialized
+GCC_WARNS += -Wunknown-pragmas -Wunsafe-loop-optimizations
+GCC_WARNS += -Wunsuffixed-float-constants -Wunused -Wunused-function
+GCC_WARNS += -Wunused-label -Wunused-local-typedefs -Wunused-macros
+GCC_WARNS += -Wunused-value -Wunused-variable -Wunused-const-variable=2
+GCC_WARNS += -Wunused-but-set-parameter -Wunused-but-set-variable
+GCC_WARNS += -Wvariadic-macros -Wvector-operation-performance -Wvla
+GCC_WARNS += -Wvolatile-register-var -Wwrite-strings
+
+# "Issue a warning when HSAIL cannot be emitted for the compiled function or
+# OpenMP construct."
+# Uh-huh.
+GCC_WARNS += -Whsa
+
+# I don't mind too much increasing these.
+# Just make sure that you know what you're doing.
+GCC_WARNS += -Wlarger-than=1024 -Walloc-size-larger-than=4096
+GCC_WARNS += -Wframe-larger-than=1024 -Wstack-usage=1024
+
+# Can't use because of dependencies: -Waggregate-return
+# Want to add, but needs work: -Wconversion, -Wsign-compare, -Wsign-conversion
+# Seem to require other compiler features: -Wchkp, -Wstack-protector,
+#     -Wstrict-aliasing, -Wstrict-overflow

diff --git a/src/address.c b/src/address.c
index 63da197154eddb16768ff83c5ed0dbcb7a1b75f2..7e6b953d1bd9914f900cef64b87dd532df011892 100644 (file)
--- a/src/address.c
+++ b/src/address.c
@@ -65,7 +65,7 @@ prefix4_decode(IPAddress2_t *str, struct ipv4_prefix *result)
        int len;
 
        if (str->size > 4) {
-               return pr_err("IPv4 address has too many octets. (%u)",
+               return pr_err("IPv4 address has too many octets. (%zu)",
                    str->size);
        }
        if (str->bits_unused < 0 || 7 < str->bits_unused) {
@@ -102,7 +102,7 @@ prefix6_decode(IPAddress2_t *str, struct ipv6_prefix *result)
        int len;
 
        if (str->size > 16) {
-               return pr_err("IPv6 address has too many octets. (%u)",
+               return pr_err("IPv6 address has too many octets. (%zu)",
                    str->size);
        }
        if (str->bits_unused < 0 || 7 < str->bits_unused) {
@@ -115,7 +115,7 @@ prefix6_decode(IPAddress2_t *str, struct ipv6_prefix *result)
        len = 8 * str->size - str->bits_unused;
 
        if (len < 0 || 128 < len) {
-               return pr_err("IPv6 prefix length (%u) is out of bounds (0-128).",
+               return pr_err("IPv6 prefix length (%d) is out of bounds (0-128).",
                    len);
        }
 

diff --git a/src/asn1/decode.c b/src/asn1/decode.c
index 087848cfa710cade3e5062e628cebe1f3c7ca9e5..2800c52d07bd0530b80aa4af6a40672bfea95f98 100644 (file)
--- a/src/asn1/decode.c
+++ b/src/asn1/decode.c
@@ -35,7 +35,7 @@ asn1_decode(const void *buffer, size_t buffer_size,
                /* Must free partial object according to API contracts. */
                ASN_STRUCT_FREE(*descriptor, *result);
                /* TODO if rval.code == RC_WMORE (1), more work is needed */
-               return pr_err("Error decoding ASN.1 object: %d", rval.code);
+               return pr_err("Error decoding ASN.1 object: %u", rval.code);
        }
 
        error = validate(descriptor, *result);

diff --git a/src/asn1/signed_data.c b/src/asn1/signed_data.c
index 5375d0ba1e0561cd14678aed8344bb64095661d5..1a0517b84f334e5d1c43f57141cc082eb4541ec9 100644 (file)
--- a/src/asn1/signed_data.c
+++ b/src/asn1/signed_data.c
@@ -18,7 +18,7 @@ static const OID oid_sta = OID_SIGNING_TIME_ATTR;
 static const OID oid_bsta = OID_BINARY_SIGNING_TIME_ATTR;
 
 static int
-is_digest_algorithm(AlgorithmIdentifier_t *id, char *what)
+is_digest_algorithm(AlgorithmIdentifier_t *id, char const *what)
 {
        bool is_hash;
        int error;
@@ -182,8 +182,7 @@ validate_signed_attrs(struct SignerInfo *sinfo, EncapsulatedContentInfo_t *eci)
                attrs = &attr->attrValues;
 
                if (attrs->list.count != 1) {
-                       return pr_err(0,
-                           "signedAttrs's attribute set size (%d) is different than 1.",
+                       return pr_err("signedAttrs's attribute set size (%d) is different than 1",
                            attr->attrValues.list.count);
                }
                if (attrs->list.array == NULL || attrs->list.array[0] == NULL)

diff --git a/src/common.c b/src/common.c
index a35c6743ef34b7c2f99237894ce5e4127285a1ae..ab2be7eaf5031ee83b9c40ed42c4fc37ddeeed2c 100644 (file)
--- a/src/common.c
+++ b/src/common.c
@@ -69,13 +69,15 @@ struct rfc5280_names {
 };
 
 static int
-get_names(X509_NAME *name, char *what, struct rfc5280_names *result)
+get_names(X509_NAME *name, char const *what, struct rfc5280_names *result)
 {
        int error;
 
        error = x509_name_decode(name, NID_commonName, &result->commonName);
-       if (error == -ESRCH)
-               return pr_err("The '%s' name lacks a commonName attribute.");
+       if (error == -ESRCH) {
+               return pr_err("The '%s' name lacks a commonName attribute.",
+                   what);
+       }
        if (error)
                return error;
 

diff --git a/src/debug.c b/src/debug.c
index 536810545d6e110c605f0223b9ba95ef0449be99..f1e325ee0ff60a6656a656924fd5d1f33cba603f 100644 (file)
--- a/src/debug.c
+++ b/src/debug.c
@@ -15,12 +15,13 @@
  */
 void print_stack_trace(void)
 {
-       void *array[256];
+#define STACK_SIZE 64
+       void *array[STACK_SIZE];
        size_t size;
        char **strings;
        size_t i;
 
-       size = backtrace(array, 256);
+       size = backtrace(array, STACK_SIZE);
        strings = backtrace_symbols(array, size);
 
        fprintf(stderr, "Stack trace:\n");

diff --git a/src/extension.c b/src/extension.c
index 18ea94547eb0959fac50e09e71f97ee07595a94f..ab7d53c727d7b8b03e40946678c472c26368fb39 100644 (file)
--- a/src/extension.c
+++ b/src/extension.c
@@ -9,13 +9,13 @@
 
 static struct extension_metadata IR2 = {
        "Amended IP Resources",
-       /* TODO NID_ipAddrBlocksv2 */ -1,
+       -1,
        true,
 };
 
 static struct extension_metadata AR2 = {
        "Amended AS Resources",
-       /* TODO NID_autonomousSysIdsv2 */ -1,
+       -1,
        true,
 };
 

diff --git a/src/extension.h b/src/extension.h
index 707c199174be681078703ce782ca5a02f1d4d676..74817ff6ee026fb736d6827b60d728bafba18494 100644 (file)
--- a/src/extension.h
+++ b/src/extension.h
@@ -5,7 +5,7 @@
 #include <openssl/x509.h>
 
 struct extension_metadata {
-       char *name;
+       char const *name;
        int nid;
        bool critical;
 };
@@ -13,11 +13,10 @@ struct extension_metadata {
 struct extension_handler {
        struct extension_metadata const *meta;
        bool mandatory;
+
        int (*cb)(X509_EXTENSION *, void *);
        void *arg;
 
-       void (*free)(void *);
-
        /* For internal use */
        bool found;
 };

diff --git a/src/log.h b/src/log.h
index fa002c178d4b70db95ce7f3899176f206c7b3997..43ead6109c27b0af2726899ab0d08a52c1a7c6e1 100644 (file)
--- a/src/log.h
+++ b/src/log.h
@@ -10,11 +10,22 @@
  *   to ease debugging.
  */
 
+#if __GNUC__
+#define CHECK_FORMAT(str, args) __attribute__((format(printf, str, args)))
+#else
+/*
+ * No idea how this looks in other compilers.
+ * It's safe to obviate since we're bound to see the warnings every time we use
+ * GCC anyway.
+ */
+#define CHECK_FORMAT(str, args) /* Nothing */
+#endif
+
 #ifdef DEBUG
 
-void pr_debug(const char *, ...);
-void pr_debug_add(const char *, ...);
-void pr_debug_rm(const char *, ...);
+void pr_debug(const char *, ...) CHECK_FORMAT(1, 2);
+void pr_debug_add(const char *, ...) CHECK_FORMAT(1, 2);
+void pr_debug_rm(const char *, ...) CHECK_FORMAT(1, 2);
 void pr_debug_prefix(void);
 
 #else
@@ -26,13 +37,12 @@ void pr_debug_prefix(void);
 
 #endif
 
-int pr_warn(const char *, ...);
-int pr_err(const char *, ...);
-int pr_errno(int, const char *, ...);
-int crypto_err(const char *, ...);
+int pr_warn(const char *, ...) CHECK_FORMAT(1, 2);
+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 *, ...);
+int pr_crit(const char *, ...) CHECK_FORMAT(1, 2);
 
 #define PR_DEBUG printf("%s:%d (%s())\n", __FILE__, __LINE__, __func__)
 #define PR_DEBUG_MSG(msg, ...) printf("%s:%d (%s()): " msg "\n", \

diff --git a/src/object/certificate.c b/src/object/certificate.c
index 34bd3afe2d92b27032df917e5c8c20f877f941fe..461457cbce91a331c558e59e7bd7f6087c84deb7 100644 (file)
--- a/src/object/certificate.c
+++ b/src/object/certificate.c
@@ -745,12 +745,12 @@ handle_ku(X509_EXTENSION *ext, unsigned char byte1)
        memcpy(data, ku->data, ku->length);
 
        if (ku->data[0] != byte1) {
-               error = pr_err("Illegal key usage flag string: %u%u%u%u%u%u%u%u%u",
-                   !!(ku->data[0] & 0x80), !!(ku->data[0] & 0x40),
-                   !!(ku->data[0] & 0x20), !!(ku->data[0] & 0x10),
-                   !!(ku->data[0] & 0x08), !!(ku->data[0] & 0x04),
-                   !!(ku->data[0] & 0x02), !!(ku->data[0] & 0x01),
-                   !!(ku->data[1] & 0x80));
+               error = pr_err("Illegal key usage flag string: %d%d%d%d%d%d%d%d%d",
+                   !!(ku->data[0] & 0x80u), !!(ku->data[0] & 0x40u),
+                   !!(ku->data[0] & 0x20u), !!(ku->data[0] & 0x10u),
+                   !!(ku->data[0] & 0x08u), !!(ku->data[0] & 0x04u),
+                   !!(ku->data[0] & 0x02u), !!(ku->data[0] & 0x01u),
+                   !!(ku->data[1] & 0x80u));
                goto end;
        }
 
@@ -781,14 +781,14 @@ handle_cdp(X509_EXTENSION *ext, void *arg)
        GENERAL_NAME *name;
        int i;
        int error = 0;
-       char *error_msg;
+       char const *error_msg;
 
        crldp = X509V3_EXT_d2i(ext);
        if (crldp == NULL)
                return cannot_decode(ext_cdp());
 
        if (sk_DIST_POINT_num(crldp) != 1) {
-               error = pr_err("The %s extension has %u distribution points. (1 expected)",
+               error = pr_err("The %s extension has %d distribution points. (1 expected)",
                    ext_cdp()->name, sk_DIST_POINT_num(crldp));
                goto end;
        }
@@ -877,7 +877,8 @@ end:
  *    are also allowed, and also supposed to be ignored.
  */
 static int
-handle_ad(char *ia_name, SIGNATURE_INFO_ACCESS *ia, char *ad_name, int ad_nid,
+handle_ad(char const *ia_name, SIGNATURE_INFO_ACCESS *ia,
+    char const *ad_name, int ad_nid,
     int (*cb)(struct rpki_uri *, void *), void *arg)
 {
        ACCESS_DESCRIPTION *ad;
@@ -1013,7 +1014,7 @@ handle_cp(X509_EXTENSION *ext, void *arg)
                return cannot_decode(ext_cp());
 
        if (sk_POLICYINFO_num(cp) != 1) {
-               error = pr_err("The %s extension has %u policy information's. (1 expected)",
+               error = pr_err("The %s extension has %d policy information's. (1 expected)",
                    ext_cp()->name, sk_POLICYINFO_num(cp));
                goto end;
        }

diff --git a/src/object/crl.c b/src/object/crl.c
index 8bd2caa77ba7710a8a35a05ce1923b840ce84ef6..059eaa141d2a3b62b1dcd864cc5cd9ce949899ea 100644 (file)
--- a/src/object/crl.c
+++ b/src/object/crl.c
@@ -117,10 +117,8 @@ crl_validate(X509_CRL *crl)
        int error;
 
        version = X509_CRL_get_version(crl);
-       if (version != 1) {
-               return pr_err("CRL version (0x%lx) is not 2 (0x%x).",
-                   version, 1);
-       }
+       if (version != 1)
+               return pr_err("CRL version (%ld) is not v2 (%d).", version, 1);
 
        if (X509_CRL_get_signature_nid(crl) != rpki_signature_algorithm())
                return pr_err("CRL's signature algorithm is not RSASSA-PKCS1-v1_5.");

diff --git a/src/resource.c b/src/resource.c
index dd3733b30a3d4426a3fb33ce0d1e0b4b4c85199f..f58a0eb972c0becc60c54eb8bdc16787103f43f4 100644 (file)
--- a/src/resource.c
+++ b/src/resource.c
@@ -52,7 +52,7 @@ int
 get_addr_family(OCTET_STRING_t *octets)
 {
        if (octets->size != 2) {
-               pr_err("Address family has %d octets. (2 expected.)",
+               pr_err("Address family has %zu octets. (2 expected.)",
                    octets->size);
                return -1;
        }
@@ -349,7 +349,7 @@ add_aors(struct resources *resources, int family,
                        break;
                case IPAddressOrRange_PR_NOTHING:
                        /* rfc3779#section-2.2.3.7 */
-                       return pr_err("Unknown IPAddressOrRange type: %d",
+                       return pr_err("Unknown IPAddressOrRange type: %u",
                            aor->present);
                }
        }
@@ -377,7 +377,7 @@ resources_add_ip(struct resources *resources, struct IPAddressFamily *obj)
        }
 
        /* rfc3779#section-2.2.3.4 */
-       return pr_err("Unknown ipAddressChoice type: %d",
+       return pr_err("Unknown ipAddressChoice type: %u",
            obj->ipAddressChoice.present);
 }
 
@@ -495,7 +495,7 @@ add_asior(struct resources *resources, struct ASIdOrRange *obj)
                return add_asn(resources, asn_min, asn_max, parent);
        }
 
-       return pr_err("Unknown ASIdOrRange type: %d", obj->present);
+       return pr_err("Unknown ASIdOrRange type: %u", obj->present);
 }
 
 int
@@ -528,7 +528,7 @@ resources_add_asn(struct resources *resources, struct ASIdentifiers *ids)
                break;
        }
 
-       return pr_err("Unknown ASIdentifierChoice: %d", ids->asnum->present);
+       return pr_err("Unknown ASIdentifierChoice: %u", ids->asnum->present);
 }
 
 bool

diff --git a/src/sorted_array.c b/src/sorted_array.c
index 8ee83c4a5a1e7d2057eeb21e964027d78577f6e0..6c29b715c1d04d8a2af1a7a16320146935b8685d 100644 (file)
--- a/src/sorted_array.c
+++ b/src/sorted_array.c
@@ -100,7 +100,7 @@ compare(struct sorted_array *sarray, void *new)
                return -EINTERSECTION;
        }
 
-       return pr_crit("Unknown comparison value: %d", cmp);
+       return pr_crit("Unknown comparison value: %u", cmp);
 }
 
 int
@@ -166,7 +166,7 @@ sarray_contains(struct sorted_array *sarray, void *elem)
                        break;
                }
 
-               pr_crit("Unknown comparison value: %d", cmp);
+               pr_crit("Unknown comparison value: %u", cmp);
                return false;
        }