This is wrong in several ways.

Never cast sizeof down, always cast the comparison variable up.

I'll specifically call out this change:

-       if (snprintf(buf, sizeof(buf)) >= (int)sizeof(buf))
+       if ((size_t)snprintf(buf, sizeof(buf)) >= sizeof(buf))

The top way fails when snprintf returns -1 (which doesn't happen, but
still) whereas the bottom way does handle it.

(yes, it's stupid that snprintf returns an int, but that's not a good
excuse. two stupids don't make a smart. :))


Index: expand.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/expand.c,v
retrieving revision 1.26
diff -u -p -r1.26 expand.c
--- expand.c    19 Apr 2014 12:43:19 -0000      1.26
+++ expand.c    8 May 2014 16:28:15 -0000
@@ -193,12 +193,14 @@ static int
 expand_line_split(char **line, char **ret)
 {
        static char     buffer[SMTPD_MAXLINESIZE];
-       int             esc, i, dq, sq;
+       int             esc, dq, sq;
+       size_t          i;
        char           *s;
 
        memset(buffer, 0, sizeof buffer);
-       esc = dq = sq = i = 0;
-       for (s = *line; (*s) && (i < (int)sizeof(buffer)); ++s) {
+       esc = dq = sq = 0;
+       i = 0;
+       for (s = *line; (*s) && (i < sizeof(buffer)); ++s) {
                if (esc) {
                        buffer[i++] = *s;
                        esc = 0;
Index: table.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/table.c,v
retrieving revision 1.15
diff -u -p -r1.15 table.c
--- table.c     19 Apr 2014 18:01:01 -0000      1.15
+++ table.c     8 May 2014 16:26:16 -0000
@@ -107,7 +107,7 @@ table_find(const char *name, const char 
        if (tag == NULL)
                return dict_get(env->sc_tables_dict, name);
 
-       if (snprintf(buf, sizeof(buf), "%s#%s", name, tag) >= (int)sizeof(buf)) 
{
+       if ((size_t)snprintf(buf, sizeof(buf), "%s#%s", name, tag) >= 
sizeof(buf)) {
                log_warnx("warn: table name too long: %s#%s", name, tag);
                return (NULL);
        }
@@ -194,8 +194,8 @@ table_create(const char *backend, const 
        struct stat              sb;
 
        if (name && tag) {
-               if (snprintf(buf, sizeof(buf), "%s#%s", name, tag)
-                   >= (int)sizeof(buf))
+               if ((size_t)snprintf(buf, sizeof(buf), "%s#%s", name, tag) >=
+                   sizeof(buf))
                        errx(1, "table_create: name too long \"%s#%s\"",
                            name, tag);
                name = buf;
@@ -205,8 +205,8 @@ table_create(const char *backend, const 
                errx(1, "table_create: table \"%s\" already defined", name);
 
        if ((tb = table_backend_lookup(backend)) == NULL) {
-               if (snprintf(path, sizeof(path), PATH_TABLES "/table-%s",
-                   backend) >= (int)sizeof(path)) {
+               if ((size_t)snprintf(path, sizeof(path), PATH_TABLES 
"/table-%s",
+                   backend) >= sizeof(path)) {
                        errx(1, "table_create: path too long \""
                            PATH_TABLES "/table-%s\"", backend);
                }
@@ -644,14 +644,14 @@ table_dump_lookup(enum table_service s, 
 
        case K_DOMAIN:
                ret = snprintf(buf, sizeof(buf), "%s", lk->domain.name);
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 
        case K_CREDENTIALS:
                ret = snprintf(buf, sizeof(buf), "%s:%s",
                    lk->creds.username, lk->creds.password);
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 
@@ -659,7 +659,7 @@ table_dump_lookup(enum table_service s, 
                ret = snprintf(buf, sizeof(buf), "%s/%d",
                    sockaddr_to_text((struct sockaddr *)&lk->netaddr.ss),
                    lk->netaddr.bits);
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 
@@ -669,14 +669,14 @@ table_dump_lookup(enum table_service s, 
                    lk->userinfo.uid,
                    lk->userinfo.gid,
                    lk->userinfo.directory);
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 
        case K_SOURCE:
                ret = snprintf(buf, sizeof(buf), "%s",
                    ss_to_text(&lk->source.addr));
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 
@@ -684,14 +684,14 @@ table_dump_lookup(enum table_service s, 
                ret = snprintf(buf, sizeof(buf), "%s@%s",
                    lk->mailaddr.user,
                    lk->mailaddr.domain);
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 
        case K_ADDRNAME:
                ret = snprintf(buf, sizeof(buf), "%s",
                    lk->addrname.name);
-               if (ret == -1 || ret >= (int)sizeof (buf))
+               if (ret == -1 || (size_t)ret >= sizeof (buf))
                        goto err;
                break;
 

Reply via email to