From: David Levine Date: Sat, 13 Sep 2014 03:12:33 +0000 (-0500) Subject: Fixed all anomalies detected by clang static analyzer (with X-Git-Url: https://diplodocus.org/git/nmh/commitdiff_plain/337b4e616e8f53ba06285b1645e1df9918ed5c16?ds=inline;hp=215f8df2be496a67942590ab2aefdad5b182fe25 Fixed all anomalies detected by clang static analyzer (with default checkers on Linux). One was notable: there was a break missing from a switch case in fmt_scan.c, which caused the result of the sday function escape to be garbage. --- diff --git a/Makefile.am b/Makefile.am index 6b6d66d2..30e02a27 100644 --- a/Makefile.am +++ b/Makefile.am @@ -52,8 +52,8 @@ TESTS = test/ali/test-ali test/anno/test-anno \ test/folder/test-create test/folder/test-packf \ test/folder/test-recurse test/folder/test-sortm \ test/folder/test-total \ - test/format/test-curses \ - test/format/test-dp test/format/test-fmtdump \ + test/format/test-curses test/format/test-dp \ + test/format/test-fmtdump test/format/test-functions \ test/format/test-localmbox test/format/test-myname \ test/format/test-myhost test/format/test-mymbox \ test/format/test-nullstr \ diff --git a/mts/smtp/smtp.c b/mts/smtp/smtp.c index 65de0a7f..58ca023c 100644 --- a/mts/smtp/smtp.c +++ b/mts/smtp/smtp.c @@ -1223,7 +1223,7 @@ sm_wstream (char *buffer, int len) return (ferror (sm_wfp) ? sm_werror () : OK); } - for (bp = buffer; len > 0; bp++, len--) { + for (bp = buffer; bp && len > 0; bp++, len--) { switch (*bp) { case '\n': sm_nl = TRUE; @@ -1467,7 +1467,7 @@ smhear (void) int i, code, cont, bc = 0, rc, more; unsigned char *bp; char *rp; - char **ehlo = NULL, buffer[BUFSIZ]; + char **ehlo = EHLOkeys, buffer[BUFSIZ]; if (doingEHLO) { static int at_least_once = 0; diff --git a/sbr/addrsbr.c b/sbr/addrsbr.c index b2079bd2..3028847a 100644 --- a/sbr/addrsbr.c +++ b/sbr/addrsbr.c @@ -436,7 +436,7 @@ local_test: ; if (mp->m_nohost) return 1; - if (np->m_host == NULL) + if (np->m_host == NULL || mp->m_host == NULL) continue; if ((len = strlen (cp = np->m_host)) < (i = strlen (pp = mp->m_host))) diff --git a/sbr/encode_rfc2047.c b/sbr/encode_rfc2047.c index 581214b7..2c8a3f0d 100644 --- a/sbr/encode_rfc2047.c +++ b/sbr/encode_rfc2047.c @@ -281,16 +281,22 @@ field_encode_quoted(const char *name, char **value, const char *charset, * allow for the encoded output. */ if (column + (utf8len(p) * 3) > ENCODELINELIMIT - 2) { - newline = 1; + newline = 1; } } } + if (q == NULL) { + /* This should never happen, but just in case. Found by + clang static analyzer. */ + admonish (NULL, "null output encoding for %s", *value); + return 1; + } *q++ = '?'; *q++ = '='; if (prefixlen) - *q++ = '\n'; + *q++ = '\n'; *q = '\0'; diff --git a/sbr/fmt_scan.c b/sbr/fmt_scan.c index cc5c192d..08405da1 100644 --- a/sbr/fmt_scan.c +++ b/sbr/fmt_scan.c @@ -499,6 +499,9 @@ fmt_scan (struct format *format, charstring_t scanlp, int width, int *dat, if (val < scale) { snprintf(buffer, sizeof(buffer), "%u", val); } else { + /* To prevent divide by 0, found by clang static + analyzer. */ + if (scale == 0) { scale = 1; } /* find correct scale for size (Kilo/Mega/Giga/Tera) */ for (unitcp = "KMGT"; val > (scale * scale); val /= scale) { @@ -821,6 +824,7 @@ fmt_scan (struct format *format, charstring_t scanlp, int width, int *dat, default: value = -1; break; } + break; case FT_LV_ZONEF: if ((fmt->f_comp->c_tws->tw_flags & TW_SZONE) == TW_SZEXP) value = 1; @@ -988,7 +992,7 @@ fmt_scan (struct format *format, charstring_t scanlp, int width, int *dat, lp = str; wid = value; - len = strlen (str); + len = str ? strlen (str) : 0; sp = fmt->f_text; indent = strlen (sp); wid -= indent; diff --git a/sbr/mime_type.c b/sbr/mime_type.c index 673642d5..06a6156d 100644 --- a/sbr/mime_type.c +++ b/sbr/mime_type.c @@ -128,7 +128,6 @@ get_file_info(const char *proc, const char *file_name) { } } - cmd = concat(proc, " ", quotec, file_name, quotec, NULL); if ((cmd = concat(proc, " ", quotec, file_name, quotec, NULL))) { FILE *fp; diff --git a/sbr/ruserpass.c b/sbr/ruserpass.c index a55f6803..7038cf5c 100644 --- a/sbr/ruserpass.c +++ b/sbr/ruserpass.c @@ -152,7 +152,7 @@ ruserpass(char *host, char **aname, char **apass) advise ("tmp", "fgets"); } tmp[strlen(tmp) - 1] = '\0'; - if (*tmp != '\0') { + if (*tmp != '\0' || myname == NULL) { myname = tmp; } diff --git a/test/format/test-functions b/test/format/test-functions new file mode 100755 index 00000000..9cfaeeeb --- /dev/null +++ b/test/format/test-functions @@ -0,0 +1,29 @@ +#!/bin/sh +# +# Test of various (well, start with one) function escapes. + +set -e + +if test -z "${MH_OBJ_DIR}"; then + srcdir=`dirname "$0"`/../.. + MH_OBJ_DIR=`cd "$srcdir" && pwd`; export MH_OBJ_DIR +fi + +. "$MH_OBJ_DIR/test/common.sh" + +setup_test +expected="$MH_TEST_DIR/$$.expected" +actual="$MH_TEST_DIR/$$.actual" + +# check sday when day of week is specified +echo 1 >"$expected" +fmttest -raw -format '%(sday{text})' 'Fri Sep 12 20:02 2014' >"$actual" +check "$expected" "$actual" + +# check sday when day of week is not specified +echo 0 >"$expected" +fmttest -raw -format '%(sday{text})' 'Sep 12 20:02 2014' >"$actual" +check "$expected" "$actual" + + +exit $failed diff --git a/uip/annosbr.c b/uip/annosbr.c index a73c8277..63d2ee44 100644 --- a/uip/annosbr.c +++ b/uip/annosbr.c @@ -117,7 +117,6 @@ annolist(char *file, char *comp, char *text, int number) for (n = 0, cp = field; (c = getc(fp)) != EOF; *cp++ = c) { if (c == '\n' && (c = getc(fp)) != ' ' && c != '\t') { (void)ungetc(c, fp); - c = '\n'; break; } diff --git a/uip/dropsbr.c b/uip/dropsbr.c index dee95fe7..66edece3 100644 --- a/uip/dropsbr.c +++ b/uip/dropsbr.c @@ -241,9 +241,9 @@ mbx_write(char *mailbox, int md, FILE *fp, int id, long last, fseek (fp, pos, SEEK_SET); while (fgets (buffer, sizeof(buffer), fp) && (pos < stop)) { i = strlen (buffer); - for (j = 0; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++) + for ( ; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++) continue; - for (j = 0; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++) + for ( ; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++) continue; if (write (md, buffer, i) != i) return NOTOK; @@ -304,13 +304,9 @@ mbx_copy (char *mailbox, int mbx_style, int md, int fd, termination. */ buffer[i] = '\0'; - for (j = 0; - (j = stringdex (mmdlm1, buffer)) >= 0; - buffer[j]++) + for ( ; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++) continue; - for (j = 0; - (j = stringdex (mmdlm2, buffer)) >= 0; - buffer[j]++) + for ( ; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++) continue; if (write (md, buffer, i) != i) return NOTOK; diff --git a/uip/fmttest.c b/uip/fmttest.c index 850e2d08..90df1e9e 100644 --- a/uip/fmttest.c +++ b/uip/fmttest.c @@ -625,7 +625,7 @@ process_single_file(FILE *in, struct msgs_array *comps, int *dat, int msgsize, * Read in the message and process the components */ - for (state = FLD;;) { + for (;;) { bufsz = sizeof(rbuf); state = m_getfld(&gstate, name, rbuf, &bufsz, in); switch (state) { @@ -650,8 +650,7 @@ process_single_file(FILE *in, struct msgs_array *comps, int *dat, int msgsize, if (fmt_findcomp("body")) { if ((i = strlen(rbuf)) < outwidth) { bufsz = min (outwidth, (int) sizeof rbuf - i); - state = m_getfld(&gstate, name, rbuf + i, - &bufsz, in); + m_getfld(&gstate, name, rbuf + i, &bufsz, in); } fmt_addcomptext("body", rbuf); diff --git a/uip/folder.c b/uip/folder.c index 98ecde68..fc183774 100644 --- a/uip/folder.c +++ b/uip/folder.c @@ -398,6 +398,7 @@ get_folder_info_body (char *fold, char *msg, boolean *crawl_children) */ if (!(mp = folder_read (fold, 1))) { admonish (NULL, "unable to read folder %s", fold); + *crawl_children = FALSE; return 0; } @@ -406,8 +407,10 @@ get_folder_info_body (char *fold, char *msg, boolean *crawl_children) retval = 0; if (fpack) { - if (folder_pack (&mp, fverb) == -1) + if (folder_pack (&mp, fverb) == -1) { + *crawl_children = FALSE; /* to please clang static analyzer */ done (1); + } seq_save (mp); /* synchronize the sequences */ context_save (); /* save the context file */ } diff --git a/uip/forw.c b/uip/forw.c index d3fd4238..12a323bb 100644 --- a/uip/forw.c +++ b/uip/forw.c @@ -509,7 +509,6 @@ mhl_draft (int out, char *digest, int volume, int issue, dup2 (pd[1], 1); close (pd[1]); - i = 1; app_msgarg(&vec, "-forwall"); app_msgarg(&vec, "-form"); app_msgarg(&vec, filter); diff --git a/uip/inc.c b/uip/inc.c index 2ee19960..1ea618db 100644 --- a/uip/inc.c +++ b/uip/inc.c @@ -969,9 +969,9 @@ pop_pack (char *s) char buffer[BUFSIZ]; snprintf (buffer, sizeof(buffer), "%s\n", s); - for (j = 0; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++) + for ( ; (j = stringdex (mmdlm1, buffer)) >= 0; buffer[j]++) continue; - for (j = 0; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++) + for ( ; (j = stringdex (mmdlm2, buffer)) >= 0; buffer[j]++) continue; fputs (buffer, pf); size += strlen (buffer) + 1; diff --git a/uip/mhbuildsbr.c b/uip/mhbuildsbr.c index 77763337..68c555d3 100644 --- a/uip/mhbuildsbr.c +++ b/uip/mhbuildsbr.c @@ -173,6 +173,7 @@ build_mime (char *infile, int autobuild, int dist, int directives, strcasecmp (name, ENCODING_FIELD) == 0) { if (autobuild) { fclose(in); + free (ct); return NULL; } else { adios (NULL, "draft shouldn't contain %s: field", name); @@ -398,7 +399,6 @@ finish_field: if ((part = (struct part *) calloc (1, sizeof(*part))) == NULL) adios (NULL, "out of memory"); *pp = part; - pp = &part->mp_next; part->mp_part = p; } @@ -780,7 +780,6 @@ use_forw: adios (NULL, "out of memory"); init_decoded_content(ct, infilename); *ctp = ct; - ci = &ct->c_ctinfo; if (get_ctinfo (buffer, ct, 0) == NOTOK) done (1); ct->c_type = CT_MESSAGE; diff --git a/uip/mhfixmsg.c b/uip/mhfixmsg.c index 5385b9ec..3180e9ce 100644 --- a/uip/mhfixmsg.c +++ b/uip/mhfixmsg.c @@ -484,9 +484,11 @@ fix_boundary (CT *ct, int *message_mods) { if ((fixed = m_mktemp2 (NULL, invo_name, NULL, &(*ct)->c_fp))) { if (replace_boundary (*ct, fixed, part_boundary) == OK) { char *filename = add ((*ct)->c_file, NULL); + CT fixed_ct; free_content (*ct); - if ((*ct = parse_mime (fixed))) { + if ((fixed_ct = parse_mime (fixed))) { + *ct = fixed_ct; (*ct)->c_unlink = 1; ++*message_mods; @@ -494,6 +496,9 @@ fix_boundary (CT *ct, int *message_mods) { report (NULL, NULL, filename, "fix multipart boundary"); } + } else { + advise (NULL, "unable to parse fixed part"); + status = NOTOK; } free (filename); } else { diff --git a/uip/mhlsbr.c b/uip/mhlsbr.c index 0c9719c2..12add6cc 100644 --- a/uip/mhlsbr.c +++ b/uip/mhlsbr.c @@ -400,7 +400,8 @@ mhl (int argc, char **argv) case SLEEPSW: if (!(cp = *argp++) || *cp == '-') adios (NULL, "missing argument to %s", argp[-2]); - sleepsw = atoi (cp);/* ZERO ok! */ + else + sleepsw = atoi (cp);/* ZERO ok! */ continue; case PROGSW: @@ -422,13 +423,13 @@ mhl (int argc, char **argv) case LENSW: if (!(cp = *argp++) || *cp == '-') adios (NULL, "missing argument to %s", argp[-2]); - if ((length = atoi (cp)) < 1) + else if ((length = atoi (cp)) < 1) adios (NULL, "bad argument %s %s", argp[-2], cp); continue; case WIDTHSW: if (!(cp = *argp++) || *cp == '-') adios (NULL, "missing argument to %s", argp[-2]); - if ((width = atoi (cp)) < 1) + else if ((width = atoi (cp)) < 1) adios (NULL, "bad argument %s %s", argp[-2], cp); continue; @@ -439,13 +440,13 @@ mhl (int argc, char **argv) case ISSUESW: if (!(cp = *argp++) || *cp == '-') adios (NULL, "missing argument to %s", argp[-2]); - if ((issue = atoi (cp)) < 1) + else if ((issue = atoi (cp)) < 1) adios (NULL, "bad argument %s %s", argp[-2], cp); continue; case VOLUMSW: if (!(cp = *argp++) || *cp == '-') adios (NULL, "missing argument to %s", argp[-2]); - if ((volume = atoi (cp)) < 1) + else if ((volume = atoi (cp)) < 1) adios (NULL, "bad argument %s %s", argp[-2], cp); continue; @@ -591,7 +592,7 @@ mhl_format (char *file, int length, int width) *cp = 0; if (*bp == ':') { - c1 = add_queue (&fmthd, &fmttl, NULL, bp + 1, CLEARTEXT); + (void) add_queue (&fmthd, &fmttl, NULL, bp + 1, CLEARTEXT); continue; } @@ -1182,15 +1183,16 @@ mcomp_format (struct mcomp *c1, struct mcomp *c2) while ((cp = getname (ap))) { if ((p = (struct pqpair *) calloc ((size_t) 1, sizeof(*p))) == NULL) adios (NULL, "unable to allocate pqpair memory"); - - if ((mp = getm (cp, NULL, 0, error, sizeof(error))) == NULL) { - p->pq_text = getcpy (cp); - p->pq_error = getcpy (error); - } else { - p->pq_text = getcpy (mp->m_text); - mnfree (mp); + else { + if ((mp = getm (cp, NULL, 0, error, sizeof(error))) == NULL) { + p->pq_text = getcpy (cp); + p->pq_error = getcpy (error); + } else { + p->pq_text = getcpy (mp->m_text); + mnfree (mp); + } + q = (q->pq_next = p); } - q = (q->pq_next = p); } for (p = pq.pq_next; p; p = q) { @@ -1237,19 +1239,20 @@ add_queue (struct mcomp **head, struct mcomp **tail, char *name, char *text, int if ((c1 = (struct mcomp *) calloc ((size_t) 1, sizeof(*c1))) == NULL) adios (NULL, "unable to allocate comp memory"); - - c1->c_flags = flags & ~INIT; - if ((c1->c_name = name ? getcpy (name) : NULL)) - c1->c_flags |= mcomp_flags (c1->c_name); - c1->c_text = text ? getcpy (text) : NULL; - if (flags & INIT) { - if (global.c_ovtxt) - c1->c_ovtxt = getcpy (global.c_ovtxt); - c1->c_offset = global.c_offset; - c1->c_ovoff = global. c_ovoff; - c1->c_width = c1->c_length = 0; - c1->c_cwidth = global.c_cwidth; - c1->c_flags |= global.c_flags & GFLAGS; + else { + c1->c_flags = flags & ~INIT; + if ((c1->c_name = name ? getcpy (name) : NULL)) + c1->c_flags |= mcomp_flags (c1->c_name); + c1->c_text = text ? getcpy (text) : NULL; + if (flags & INIT) { + if (global.c_ovtxt) + c1->c_ovtxt = getcpy (global.c_ovtxt); + c1->c_offset = global.c_offset; + c1->c_ovoff = global. c_ovoff; + c1->c_width = c1->c_length = 0; + c1->c_cwidth = global.c_cwidth; + c1->c_flags |= global.c_flags & GFLAGS; + } } if (*head == NULL) *head = c1; diff --git a/uip/mhparse.c b/uip/mhparse.c index 14701398..e7782f14 100644 --- a/uip/mhparse.c +++ b/uip/mhparse.c @@ -2341,6 +2341,7 @@ openExternal (CT ct, CT cb, CE ce, char **file, int *fd) } } + *fd = fileno (ce->ce_fp); return OK; ready_already: diff --git a/uip/mhshowsbr.c b/uip/mhshowsbr.c index 61f04120..720ba9bc 100644 --- a/uip/mhshowsbr.c +++ b/uip/mhshowsbr.c @@ -135,7 +135,7 @@ show_single_message (CT ct, char *form, int concatsw, int textonly, { sigset_t set, oset; - int status; + int status = OK; /* Allow user executable bit so that temporary directories created by * the viewer (e.g., lynx) are going to be accessible */ @@ -329,7 +329,7 @@ show_content_aux (CT ct, int alternate, char *cp, char *cracked, struct format * { int fd; int xstdin = 0, xlist = 0; - char *file, buffer[BUFSIZ]; + char *file = NULL, buffer[BUFSIZ]; if (!ct->c_ceopenfnx) { if (!alternate) @@ -338,7 +338,6 @@ show_content_aux (CT ct, int alternate, char *cp, char *cracked, struct format * return NOTOK; } - file = NULL; if ((fd = (*ct->c_ceopenfnx) (ct, &file)) == NOTOK) return NOTOK; if (ct->c_showproc && !strcmp (ct->c_showproc, "true")) @@ -643,7 +642,7 @@ show_multi_aux (CT ct, int alternate, char *cp, struct format *fmt) /* xstdin is only used in the call to parse_display_string(): its value is ignored in the function. */ int xstdin = 0, xlist = 0; - char *file, buffer[BUFSIZ]; + char *file = NULL, buffer[BUFSIZ]; struct multipart *m = (struct multipart *) ct->c_ctparams; struct part *part; CT p; @@ -658,7 +657,6 @@ show_multi_aux (CT ct, int alternate, char *cp, struct format *fmt) } if (p->c_storage == NULL) { - file = NULL; if ((*p->c_ceopenfnx) (p, &file) == NOTOK) return NOTOK; diff --git a/uip/picksbr.c b/uip/picksbr.c index 74834f4e..d444cd68 100644 --- a/uip/picksbr.c +++ b/uip/picksbr.c @@ -243,6 +243,7 @@ parse (void) if ((o->n_R_child = parse ())) return o; padvise (NULL, "missing disjunctive"); + free (o); return NULL; header: ; @@ -263,6 +264,7 @@ nexp1 (void) if (*cp != '-') { padvise (NULL, "%s unexpected", cp); + free (n); return NULL; } @@ -272,10 +274,12 @@ nexp1 (void) case AMBIGSW: ambigsw (cp, parswit); talked++; + free (n); return NULL; case UNKWNSW: fprintf (stderr, "-%s unknown\n", cp); talked++; + free (n); return NULL; case PRAND: @@ -284,6 +288,7 @@ nexp1 (void) if ((o->n_R_child = nexp1 ())) return o; padvise (NULL, "missing conjunctive"); + free (o); return NULL; header: ; @@ -325,6 +330,7 @@ nexp2 (void) if ((n->n_L_child = nexp3 ())) return n; padvise (NULL, "missing negation"); + free (n); return NULL; header: ; @@ -406,12 +412,14 @@ nexp3 (void) n->n_header = 0; if (!(cp = nxtarg ())) {/* allow -xyz arguments */ padvise (NULL, "missing argument to %s", argp[-2]); + free (n); return NULL; } dp = cp; pattern: ; if (!gcompile (n, dp)) { padvise (NULL, "pattern error in %s %s", argp[-2], cp); + free (n); return NULL; } n->n_patbuf = getcpy (dp); @@ -438,6 +446,7 @@ nexp3 (void) n->n_datef = datesw; if (!tcompile (cp, &n->n_tws, n->n_after = i == PRAFTR)) { padvise (NULL, "unable to parse %s %s", argp[-2], cp); + free (n); return NULL; } return n; diff --git a/uip/popsbr.c b/uip/popsbr.c index 8334dee6..2d194a67 100644 --- a/uip/popsbr.c +++ b/uip/popsbr.c @@ -899,7 +899,7 @@ sasl_getline (char *s, int n, FILE *iop) *p = 0; if (*--p == '\n') *p = 0; - if (*--p == '\r') + if (p > s && *--p == '\r') *p = 0; return OK; diff --git a/uip/post.c b/uip/post.c index 16121359..685129b9 100644 --- a/uip/post.c +++ b/uip/post.c @@ -714,7 +714,7 @@ putfmt (char *name, char *str, FILE *out) if (hdr->flags & HFCC) { if ((cp = strrchr(str, '\n'))) *cp = 0; - for (cp = pp = str; (cp = strchr(pp, ',')); pp = cp) { + for (pp = str; (cp = strchr(pp, ',')); pp = cp) { *cp++ = 0; insert_fcc (hdr, pp); } diff --git a/uip/scansbr.c b/uip/scansbr.c index ce49bee2..749743c1 100644 --- a/uip/scansbr.c +++ b/uip/scansbr.c @@ -220,7 +220,6 @@ scan (FILE *inb, int innum, int outnum, char *nfs, int width, int curflg, break; case BODY: - compnum = -1; /* * A slight hack ... if we have less than rlwidth characters * in the buffer, call m_getfld again.