Skip to content

Static analysis cleanup: dead code, format string, size_t nits - #426

Merged
thegushi merged 1 commit into
trusteddomainproject:developfrom
thegushi:fix/static-analysis-cleanup
Jul 22, 2026
Merged

Static analysis cleanup: dead code, format string, size_t nits#426
thegushi merged 1 commit into
trusteddomainproject:developfrom
thegushi:fix/static-analysis-cleanup

Conversation

@thegushi

Copy link
Copy Markdown
Collaborator

Summary

A grab-bag of small cppcheck findings, none independently significant but worth cleaning up together. Split out from #424 and #425, which cover the two higher-priority findings from the same static analysis pass.

  • 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 its 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).

Test plan

  • autoreconf -fi && ./configure && gmake builds clean
  • gmake check: 174/174 tests pass

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).
@thegushi
thegushi merged commit bfacdd8 into trusteddomainproject:develop Jul 22, 2026
2 checks passed
@thegushi
thegushi deleted the fix/static-analysis-cleanup branch July 22, 2026 02:16
thegushi added a commit that referenced this pull request Jul 22, 2026
thegushi added a commit that referenced this pull request Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant