Skip to content

Commit b18829e

Browse files
committed
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).
1 parent b1179c8 commit b18829e

5 files changed

Lines changed: 21 additions & 37 deletions

File tree

libopendkim/base32.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ dkim_base32_encode(char *buf, size_t *buflen, const void *data, size_t size)
6969
buf[iout] = cb32[((udata[iin] & 0xf8) >> 3)];
7070
iout++;
7171

72-
if (iout >= *buflen || iin >= size)
72+
if (iout >= *buflen)
7373
{
7474
iout--; /* previous char is useless */
7575
break;
@@ -86,7 +86,7 @@ dkim_base32_encode(char *buf, size_t *buflen, const void *data, size_t size)
8686
buf[iout] = cb32[((udata[iin] & 0x3e) >> 1)];
8787
iout++;
8888

89-
if (iout >= *buflen || iin >= size)
89+
if (iout >= *buflen)
9090
{
9191
iout--; /* previous char is useless */
9292
break;
@@ -110,7 +110,7 @@ dkim_base32_encode(char *buf, size_t *buflen, const void *data, size_t size)
110110
buf[iout] = cb32[((udata[iin] & 0x7c) >> 2)];
111111
iout++;
112112

113-
if (iout >= *buflen || iin >= size)
113+
if (iout >= *buflen)
114114
{
115115
iout--; /* previous char is useless */
116116
break;

libopendkim/dkim-test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ dkim_test_key2(DKIM_LIB *lib, char *selector, char *domain,
507507
{
508508
status = 1;
509509
snprintf(err, errlen,
510-
"key do not match: local = %zd, remote = %zd",
510+
"keys do not match: local = %zu, remote = %zu",
511511
outkey_len, sig->sig_keylen);
512512
}
513513

libopendkim/dkim.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,7 @@ dkim_process_set(DKIM *dkim, dkim_set_t type, u_char *str, size_t len,
872872
if (value != NULL)
873873
{
874874
uint64_t tmp = 0;
875-
char *end;
875+
char *end = NULL;
876876

877877
errno = 0;
878878

@@ -909,7 +909,7 @@ dkim_process_set(DKIM *dkim, dkim_set_t type, u_char *str, size_t len,
909909
if (value != NULL)
910910
{
911911
uint64_t tmp = 0;
912-
char *end;
912+
char *end = NULL;
913913

914914
errno = 0;
915915

@@ -5425,6 +5425,7 @@ dkim_sign(DKIM_LIB *libhandle, const unsigned char *id, void *memclosure,
54255425
if (strncmp((char *) secretkey, "MII", 3) == 0)
54265426
{
54275427
size_t b64len;
5428+
int declen;
54285429

54295430
b64len = strlen((char *) secretkey);
54305431

@@ -5437,15 +5438,16 @@ dkim_sign(DKIM_LIB *libhandle, const unsigned char *id, void *memclosure,
54375438
return NULL;
54385439
}
54395440

5440-
new->dkim_keylen = dkim_base64_decode(secretkey,
5441-
new->dkim_key,
5442-
b64len);
5443-
if (new->dkim_keylen <= 0)
5441+
declen = dkim_base64_decode(secretkey, new->dkim_key,
5442+
b64len);
5443+
if (declen <= 0)
54445444
{
54455445
*statp = DKIM_STAT_NORESOURCE;
54465446
dkim_free(new);
54475447
return NULL;
54485448
}
5449+
5450+
new->dkim_keylen = (size_t) declen;
54495451
}
54505452
else
54515453
{
@@ -7541,14 +7543,12 @@ dkim_getsighdr_d(DKIM *dkim, size_t initial, u_char **buf, size_t *buflen)
75417543
}
75427544
else
75437545
{
7544-
if (!first)
7545-
{
7546-
dkim_dstring_cat1(dkim->dkim_hdrbuf,
7547-
' ');
7548-
len += 1;
7549-
}
7546+
/* "first" is always FALSE here: this branch is
7547+
only reached when the "len == 0 || first"
7548+
branch above was not taken */
7549+
dkim_dstring_cat1(dkim->dkim_hdrbuf, ' ');
7550+
len += 1;
75507551

7551-
first = FALSE;
75527552
dkim_dstring_catn(dkim->dkim_hdrbuf,
75537553
(u_char *) pv,
75547554
pvlen);

librbl/rbl.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,8 @@ rbl_res_query(void *srv, int type, unsigned char *query, unsigned char *buf,
183183
if (rq == NULL)
184184
return RBL_DNS_ERROR;
185185

186-
if (ret == -1)
187-
{
188-
rq->rq_error = errno;
189-
rq->rq_buflen = 0;
190-
}
191-
else
192-
{
193-
rq->rq_error = 0;
194-
rq->rq_buflen = (size_t) ret;
195-
}
186+
rq->rq_error = 0;
187+
rq->rq_buflen = (size_t) ret;
196188

197189
*qh = (void *) rq;
198190

libvbr/vbr.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -487,16 +487,8 @@ vbr_res_query(void *srv, int type, unsigned char *query, unsigned char *buf,
487487
if (vq == NULL)
488488
return VBR_DNS_ERROR;
489489

490-
if (ret == -1)
491-
{
492-
vq->vq_error = errno;
493-
vq->vq_buflen = 0;
494-
}
495-
else
496-
{
497-
vq->vq_error = 0;
498-
vq->vq_buflen = (size_t) ret;
499-
}
490+
vq->vq_error = 0;
491+
vq->vq_buflen = (size_t) ret;
500492

501493
*qh = (void *) vq;
502494

0 commit comments

Comments
 (0)