From b18829e114c9563abe944b7ad7744b2e489eb332 Mon Sep 17 00:00:00 2001 From: Dan Mahoney Date: Tue, 21 Jul 2026 18:58:42 -0700 Subject: [PATCH] Static analysis cleanup: dead code, format string, size_t nits A grab-bag of small cppcheck findings, none independently significant but worth cleaning up together: - libvbr/vbr.c, librbl/rbl.c: remove a dead "if (ret == -1) {...} else {...}" block in the DNS query path. An identical "ret == -1" check earlier in the same function already returns on that condition, so the else branch always ran; cppcheck confirms both conditions are always false. - libopendkim/dkim-test.c: fix %zd/%zu format string mismatch (outkey_len and sig_keylen are size_t, not ssize_t) and a "key do not match" -> "keys do not match" typo to match nearby wording. - libopendkim/dkim.c, dkim_sign(): dkim_base64_decode() returns int (-1 on error) but was assigned directly into dkim_keylen, which is size_t; "dkim_keylen <= 0" never caught the -1 case since it wraps to SIZE_MAX. Not reachable via opendkim-genkey (always produces PEM), but a real hazard for any direct libopendkim caller passing a bare base64-DER key. Now captured in an int local and checked before assigning into the size_t field. - libopendkim/dkim.c, dkim_process_set(): initialize "end" in the "t" and "x" tag validation so it's not left uninitialized on paths where it's currently only safe because of short-circuit evaluation. - libopendkim/dkim.c, dkim_getsighdr_d(): simplify a dead "if (!first)" check in the tag-wrapping loop -- by the time that branch is reached, "first" is already guaranteed FALSE (the branch above already handles "len == 0 || first"), so the check and the trailing reassignment were both no-ops. cppcheck: "Condition '!first' is always true". - libopendkim/base32.c, dkim_base32_encode(): remove a redundant "iin >= size" clause from three of the unrolled loop's bounds checks, each of which immediately re-tests a value already proven false by an identical check a few lines above with no intervening change to "iin". cppcheck: "Condition 'iin>=size' is always false" (x3). --- libopendkim/base32.c | 6 +++--- libopendkim/dkim-test.c | 2 +- libopendkim/dkim.c | 26 +++++++++++++------------- librbl/rbl.c | 12 ++---------- libvbr/vbr.c | 12 ++---------- 5 files changed, 21 insertions(+), 37 deletions(-) diff --git a/libopendkim/base32.c b/libopendkim/base32.c index f0ca9cd8..b17b325f 100644 --- a/libopendkim/base32.c +++ b/libopendkim/base32.c @@ -69,7 +69,7 @@ dkim_base32_encode(char *buf, size_t *buflen, const void *data, size_t size) buf[iout] = cb32[((udata[iin] & 0xf8) >> 3)]; iout++; - if (iout >= *buflen || iin >= size) + if (iout >= *buflen) { iout--; /* previous char is useless */ break; @@ -86,7 +86,7 @@ dkim_base32_encode(char *buf, size_t *buflen, const void *data, size_t size) buf[iout] = cb32[((udata[iin] & 0x3e) >> 1)]; iout++; - if (iout >= *buflen || iin >= size) + if (iout >= *buflen) { iout--; /* previous char is useless */ break; @@ -110,7 +110,7 @@ dkim_base32_encode(char *buf, size_t *buflen, const void *data, size_t size) buf[iout] = cb32[((udata[iin] & 0x7c) >> 2)]; iout++; - if (iout >= *buflen || iin >= size) + if (iout >= *buflen) { iout--; /* previous char is useless */ break; diff --git a/libopendkim/dkim-test.c b/libopendkim/dkim-test.c index e228d232..e7684565 100644 --- a/libopendkim/dkim-test.c +++ b/libopendkim/dkim-test.c @@ -507,7 +507,7 @@ dkim_test_key2(DKIM_LIB *lib, char *selector, char *domain, { status = 1; snprintf(err, errlen, - "key do not match: local = %zd, remote = %zd", + "keys do not match: local = %zu, remote = %zu", outkey_len, sig->sig_keylen); } diff --git a/libopendkim/dkim.c b/libopendkim/dkim.c index 542837fd..6e180141 100644 --- a/libopendkim/dkim.c +++ b/libopendkim/dkim.c @@ -872,7 +872,7 @@ dkim_process_set(DKIM *dkim, dkim_set_t type, u_char *str, size_t len, if (value != NULL) { uint64_t tmp = 0; - char *end; + char *end = NULL; errno = 0; @@ -909,7 +909,7 @@ dkim_process_set(DKIM *dkim, dkim_set_t type, u_char *str, size_t len, if (value != NULL) { uint64_t tmp = 0; - char *end; + char *end = NULL; errno = 0; @@ -5425,6 +5425,7 @@ dkim_sign(DKIM_LIB *libhandle, const unsigned char *id, void *memclosure, if (strncmp((char *) secretkey, "MII", 3) == 0) { size_t b64len; + int declen; b64len = strlen((char *) secretkey); @@ -5437,15 +5438,16 @@ dkim_sign(DKIM_LIB *libhandle, const unsigned char *id, void *memclosure, return NULL; } - new->dkim_keylen = dkim_base64_decode(secretkey, - new->dkim_key, - b64len); - if (new->dkim_keylen <= 0) + declen = dkim_base64_decode(secretkey, new->dkim_key, + b64len); + if (declen <= 0) { *statp = DKIM_STAT_NORESOURCE; dkim_free(new); return NULL; } + + new->dkim_keylen = (size_t) declen; } else { @@ -7541,14 +7543,12 @@ dkim_getsighdr_d(DKIM *dkim, size_t initial, u_char **buf, size_t *buflen) } else { - if (!first) - { - dkim_dstring_cat1(dkim->dkim_hdrbuf, - ' '); - len += 1; - } + /* "first" is always FALSE here: this branch is + only reached when the "len == 0 || first" + branch above was not taken */ + dkim_dstring_cat1(dkim->dkim_hdrbuf, ' '); + len += 1; - first = FALSE; dkim_dstring_catn(dkim->dkim_hdrbuf, (u_char *) pv, pvlen); diff --git a/librbl/rbl.c b/librbl/rbl.c index 3b675c34..b3992bdb 100644 --- a/librbl/rbl.c +++ b/librbl/rbl.c @@ -183,16 +183,8 @@ rbl_res_query(void *srv, int type, unsigned char *query, unsigned char *buf, if (rq == NULL) return RBL_DNS_ERROR; - if (ret == -1) - { - rq->rq_error = errno; - rq->rq_buflen = 0; - } - else - { - rq->rq_error = 0; - rq->rq_buflen = (size_t) ret; - } + rq->rq_error = 0; + rq->rq_buflen = (size_t) ret; *qh = (void *) rq; diff --git a/libvbr/vbr.c b/libvbr/vbr.c index 6580ce3a..e250f307 100644 --- a/libvbr/vbr.c +++ b/libvbr/vbr.c @@ -487,16 +487,8 @@ vbr_res_query(void *srv, int type, unsigned char *query, unsigned char *buf, if (vq == NULL) return VBR_DNS_ERROR; - if (ret == -1) - { - vq->vq_error = errno; - vq->vq_buflen = 0; - } - else - { - vq->vq_error = 0; - vq->vq_buflen = (size_t) ret; - } + vq->vq_error = 0; + vq->vq_buflen = (size_t) ret; *qh = (void *) vq;