I was looking at mandoc and noticed it has too many strlcats (a common
affliction affecting quite a few programs.) It's faster and simpler to
use snprintf.

The code in roff.c was doing something twisty with the length argument
to strlcpy. Doing fancy length tricks kind of defeats the purpose of
having a simple to use function like strlcpy. I believe the intention
was to only copy part of the string, so I have retained that feature.

Index: html.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/html.c,v
retrieving revision 1.30
diff -u -p -r1.30 html.c
--- html.c      28 May 2012 13:00:51 -0000      1.30
+++ html.c      23 May 2013 20:52:43 -0000
@@ -618,10 +618,7 @@ void
 bufcat_style(struct html *h, const char *key, const char *val)
 {
 
-       bufcat(h, key);
-       bufcat(h, ":");
-       bufcat(h, val);
-       bufcat(h, ";");
+       bufcat_fmt(h, "%s:%s;", key, val);
 }
 
 void
Index: man_html.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/man_html.c,v
retrieving revision 1.48
diff -u -p -r1.48 man_html.c
--- man_html.c  17 Nov 2012 00:25:20 -0000      1.48
+++ man_html.c  23 May 2013 20:50:52 -0000
@@ -301,9 +301,10 @@ man_root_pre(MAN_ARGS)
        struct tag      *t, *tt;
        char             b[BUFSIZ], title[BUFSIZ];
 
-       b[0] = 0;
        if (man->vol)
-               (void)strlcat(b, man->vol, BUFSIZ);
+               strlcpy(b, man->vol, sizeof(b));
+       else
+               b[0] = '\0';
 
        assert(man->title);
        assert(man->msec);
Index: mandocdb.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mandocdb.c,v
retrieving revision 1.42
diff -u -p -r1.42 mandocdb.c
--- mandocdb.c  24 May 2012 23:33:23 -0000      1.42
+++ mandocdb.c  23 May 2013 20:23:31 -0000
@@ -286,7 +286,6 @@ mandocdb(int argc, char *argv[])
        int              ch, i, flags;
        DB              *hash; /* temporary keyword hashtable */
        BTREEINFO        info; /* btree configuration */
-       size_t           sz1, sz2;
        struct buf       buf, /* keyword buffer */
                         dbuf; /* description buffer */
        struct of       *of; /* list of files for processing */
@@ -397,19 +396,12 @@ mandocdb(int argc, char *argv[])
                        perror(dir);
                        exit((int)MANDOCLEVEL_BADARG);
                }
-               if (strlcat(pbuf, "/", PATH_MAX) >= PATH_MAX) {
-                       fprintf(stderr, "%s: path too long\n", pbuf);
-                       exit((int)MANDOCLEVEL_BADARG);
-               }
-
-               strlcat(mdb.dbn, pbuf, MAXPATHLEN);
-               sz1 = strlcat(mdb.dbn, MANDOC_DB, MAXPATHLEN);
-
-               strlcat(mdb.idxn, pbuf, MAXPATHLEN);
-               sz2 = strlcat(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
 
-               if (sz1 >= MAXPATHLEN || sz2 >= MAXPATHLEN) {
-                       fprintf(stderr, "%s: path too long\n", mdb.idxn);
+               if ((size_t)snprintf(mdb.dbn, sizeof(mdb.dbn), "%s/%s", pbuf,
+                   MANDOC_DB) >= sizeof(mdb.dbn) ||
+                   (size_t)snprintf(mdb.idxn, sizeof(mdb.idxn), "%s/%s", pbuf,
+                   MANDOC_IDX) >= sizeof(mdb.dbn)) {
+                       fprintf(stderr, "%s: path too long\n", mdb.dbn);
                        exit((int)MANDOCLEVEL_BADARG);
                }
 
@@ -486,8 +478,8 @@ mandocdb(int argc, char *argv[])
 
                flags = O_CREAT | O_EXCL | O_RDWR;
                while (NULL == mdb.db) {
-                       strlcpy(mdb.dbn, MANDOC_DB, MAXPATHLEN);
-                       strlcat(mdb.dbn, ".XXXXXXXXXX", MAXPATHLEN);
+                       snprintf(mdb.dbn, sizeof(mdb.dbn), "%s.XXXXXXXXXX",
+                           MANDOC_DB);
                        if (NULL == mktemp(mdb.dbn)) {
                                perror(mdb.dbn);
                                exit((int)MANDOCLEVEL_SYSERR);
@@ -500,8 +492,8 @@ mandocdb(int argc, char *argv[])
                        }
                }
                while (NULL == mdb.idx) {
-                       strlcpy(mdb.idxn, MANDOC_IDX, MAXPATHLEN);
-                       strlcat(mdb.idxn, ".XXXXXXXXXX", MAXPATHLEN);
+                       snprintf(mdb.idxn, sizeof(mdb.idxn), "%s.XXXXXXXXXX",
+                           MANDOC_IDX);
                        if (NULL == mktemp(mdb.idxn)) {
                                perror(mdb.idxn);
                                unlink(mdb.dbn);
@@ -1809,12 +1801,8 @@ ofile_dirbuild(const char *dir, const ch
                                        continue;
                        }
 
-                       buf[0] = '\0';
-                       strlcat(buf, dir, MAXPATHLEN);
-                       strlcat(buf, "/", MAXPATHLEN);
-                       sz = strlcat(buf, fn, MAXPATHLEN);
-
-                       if (MAXPATHLEN <= sz) {
+                       if ((size_t)snprintf(buf, sizeof(buf), "%s/%s", dir,
+                           fn) >= sizeof(buf)) {
                                if (warnings) fprintf(stderr, "%s/%s: "
                                    "path too long\n", dir, fn);
                                continue;
@@ -1882,10 +1870,8 @@ ofile_dirbuild(const char *dir, const ch
                 * when they used to have source manuals before,
                 * and in ports, old manuals get removed on update.
                 */
-               if (0 == use_all && MANDOC_FORM & src_form &&
-                               '\0' != *psec) {
-                       buf[0] = '\0';
-                       strlcat(buf, dir, MAXPATHLEN);
+               if (0 == use_all && MANDOC_FORM & src_form && '\0' != *psec) {
+                       strlcpy(buf, dir, MAXPATHLEN);
                        p = strrchr(buf, '/');
                        if ('\0' != *parch && NULL != p)
                                for (p--; p > buf; p--)
@@ -1920,12 +1906,11 @@ ofile_dirbuild(const char *dir, const ch
                        }
                }
 
-               buf[0] = '\0';
                assert('.' == dir[0]);
-               if ('/' == dir[1]) {
-                       strlcat(buf, dir + 2, MAXPATHLEN);
-                       strlcat(buf, "/", MAXPATHLEN);
-               }
+               if ('/' == dir[1])
+                       snprintf(buf, sizeof(buf), "%s/", dir + 2);
+               else
+                       buf[0] = '\0';
                sz = strlcat(buf, fn, MAXPATHLEN);
                if (sz >= MAXPATHLEN) {
                        if (warnings) fprintf(stderr,
Index: mdoc_html.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc_html.c,v
retrieving revision 1.67
diff -u -p -r1.67 mdoc_html.c
--- mdoc_html.c 17 Nov 2012 00:25:20 -0000      1.67
+++ mdoc_html.c 23 May 2013 20:33:25 -0000
@@ -525,15 +525,12 @@ mdoc_root_pre(MDOC_ARGS)
        struct tag      *t, *tt;
        char             b[BUFSIZ], title[BUFSIZ];
 
-       strlcpy(b, meta->vol, BUFSIZ);
+       if (meta->arch)
+               snprintf(b, sizeof(b), "%s (%s)", meta->vol, meta->arch);
+       else
+               snprintf(b, sizeof(b), "%s", meta->vol);
 
-       if (meta->arch) {
-               strlcat(b, " (", BUFSIZ);
-               strlcat(b, meta->arch, BUFSIZ);
-               strlcat(b, ")", BUFSIZ);
-       }
-
-       snprintf(title, BUFSIZ - 1, "%s(%s)", meta->title, meta->msec);
+       snprintf(title, sizeof(title), "%s(%s)", meta->title, meta->msec);
 
        PAIR_SUMMARY_INIT(&tag[0], "Document Header");
        PAIR_CLASS_INIT(&tag[1], "head");
@@ -1016,8 +1013,7 @@ mdoc_bl_pre(MDOC_ARGS)
        PAIR_STYLE_INIT(&tag[0], h);
 
        assert(lists[n->norm->Bl.type]);
-       strlcpy(buf, "list ", BUFSIZ);
-       strlcat(buf, lists[n->norm->Bl.type], BUFSIZ);
+       snprintf(buf, sizeof(buf), "list %s", lists[n->norm->Bl.type]);
        PAIR_INIT(&tag[1], ATTR_CLASS, buf);
 
        /* Set the block's left-hand margin. */
Index: mdoc_validate.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc_validate.c,v
retrieving revision 1.109
diff -u -p -r1.109 mdoc_validate.c
--- mdoc_validate.c     17 Nov 2012 00:25:20 -0000      1.109
+++ mdoc_validate.c     23 May 2013 20:32:42 -0000
@@ -1241,8 +1241,7 @@ post_at(POST_ARGS)
                q = mdoc->last->child->string;
                sz = strlen(p) + strlen(q) + 1;
                buf = mandoc_malloc(sz);
-               strlcpy(buf, p, sz);
-               strlcat(buf, q, sz);
+               snprintf(buf, sz, "%s%s", p, q);
                free(mdoc->last->child->string);
                mdoc->last->child->string = buf;
        }
@@ -2288,7 +2287,7 @@ post_os(POST_ARGS)
                        return(1);
                }
 #ifdef OSNAME
-               if (strlcat(buf, OSNAME, BUFSIZ) >= BUFSIZ) {
+               if (strlcpy(buf, OSNAME, sizeof(buf)) >= sizeof(buf)) {
                        mdoc_nmsg(mdoc, n, MANDOCERR_MEM);
                        return(0);
                }
@@ -2299,15 +2298,8 @@ post_os(POST_ARGS)
                         return(post_prol(mdoc));
                 }
 
-               if (strlcat(buf, utsname.sysname, BUFSIZ) >= BUFSIZ) {
-                       mdoc_nmsg(mdoc, n, MANDOCERR_MEM);
-                       return(0);
-               }
-               if (strlcat(buf, " ", BUFSIZ) >= BUFSIZ) {
-                       mdoc_nmsg(mdoc, n, MANDOCERR_MEM);
-                       return(0);
-               }
-               if (strlcat(buf, utsname.release, BUFSIZ) >= BUFSIZ) {
+               if ((size_t)snprintf(buf, sizeof(buf), "%s %s",
+                   utsname.sysname, utsname.release) >= sizeof(buf)) {
                        mdoc_nmsg(mdoc, n, MANDOCERR_MEM);
                        return(0);
                }
Index: roff.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/roff.c,v
retrieving revision 1.48
diff -u -p -r1.48 roff.c
--- roff.c      7 Jul 2012 18:27:36 -0000       1.48
+++ roff.c      23 May 2013 21:02:02 -0000
@@ -18,6 +18,7 @@
 #include <assert.h>
 #include <ctype.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>
 
 #include "mandoc.h"
@@ -572,9 +573,8 @@ again:
                nsz = *szp + strlen(res) + 1;
                n = mandoc_malloc(nsz);
 
-               strlcpy(n, *bufp, (size_t)(stesc - *bufp + 1));
-               strlcat(n, res, nsz);
-               strlcat(n, cp + (maxl ? 0 : 1), nsz);
+               snprintf(n, nsz, "%.*s%s%s", (int)(stesc - *bufp + 1),
+                   *bufp, res, cp + (maxl ? 0 : 1));
 
                free(*bufp);
 
@@ -1567,9 +1567,8 @@ roff_userdef(ROFF_ARGS)
                *szp = strlen(n1) - 3 + strlen(arg[i]) + 1;
                n2 = mandoc_malloc(*szp);
 
-               strlcpy(n2, n1, (size_t)(cp - n1 + 1));
-               strlcat(n2, arg[i], *szp);
-               strlcat(n2, cp + 3, *szp);
+               snprintf(n2, *szp, "%.*s%s%s", (int)(cp - n1 + 1),
+                   n1, arg[i], cp + 3);
 
                cp = n2 + (cp - n1);
                free(n1);

Reply via email to