On Fri, Aug 22, 2014 at 09:14:33PM +0200, Alexander Bluhm wrote:
> Hi,
>
> When compiling syslogd with WARNINGS=yes gcc complains with many
> "warning: comparison between signed and unsigned".
>
> I would like to fix them.
>
> ok?
I still need an ok. Note that some checks got stricter.
The (size_t) cast is only done, if the argument is not negative.
I have another bugfix on top of this diff.
bluhm
>
> bluhm
>
> Index: usr.sbin/syslogd/privsep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
> retrieving revision 1.40
> diff -u -p -r1.40 privsep.c
> --- usr.sbin/syslogd/privsep.c 21 Aug 2014 17:00:34 -0000 1.40
> +++ usr.sbin/syslogd/privsep.c 22 Aug 2014 18:52:12 -0000
> @@ -330,7 +330,7 @@ priv_init(char *conf, int numeric, int l
> errx(1, "rejected attempt to getnameinfo");
> /* Expecting: length, sockaddr */
> must_read(socks[0], &addr_len, sizeof(int));
> - if (addr_len <= 0 || addr_len > sizeof(addr))
> + if (addr_len <= 0 || (size_t)addr_len > sizeof(addr))
> _exit(1);
> must_read(socks[0], &addr, addr_len);
> if (getnameinfo((struct sockaddr *)&addr, addr_len,
> @@ -459,7 +459,7 @@ check_tty_name(char *tty, size_t ttylen)
> char *p;
>
> /* Any path containing '..' is invalid. */
> - for (p = tty; *p && (p - tty) < ttylen; p++)
> + for (p = tty; *p && p < tty + ttylen; p++)
> if (*p == '.' && *(p + 1) == '.')
> goto bad_path;
>
> @@ -484,7 +484,7 @@ check_log_name(char *lognam, size_t logl
> char *p;
>
> /* Any path containing '..' is invalid. */
> - for (p = lognam; *p && (p - lognam) < loglen; p++)
> + for (p = lognam; *p && p < lognam + loglen; p++)
> if (*p == '.' && *(p + 1) == '.')
> goto bad_path;
>
> @@ -693,7 +693,7 @@ priv_getaddrinfo(char *host, char *serv,
> return (-1);
>
> /* Make sure we aren't overflowing the passed in buffer */
> - if (addr_len < ret_len)
> + if (ret_len < 0 || (size_t)ret_len > addr_len)
> errx(1, "%s: overflow attempt in return", __func__);
>
> /* Read the resolved address and make sure we got all of it */
> @@ -727,7 +727,7 @@ priv_getnameinfo(struct sockaddr *sa, so
> return (-1);
>
> /* Check we don't overflow the passed in buffer */
> - if (hostlen < ret_len)
> + if (ret_len < 0 || (size_t)ret_len > hostlen)
> errx(1, "%s: overflow attempt in return", __func__);
>
> /* Read the resolved hostname */
> @@ -767,7 +767,8 @@ static int
> may_read(int fd, void *buf, size_t n)
> {
> char *s = buf;
> - ssize_t res, pos = 0;
> + ssize_t res;
> + size_t pos = 0;
>
> while (n > pos) {
> res = read(fd, s + pos, n - pos);
> @@ -790,7 +791,8 @@ static void
> must_read(int fd, void *buf, size_t n)
> {
> char *s = buf;
> - ssize_t res, pos = 0;
> + ssize_t res;
> + size_t pos = 0;
>
> while (n > pos) {
> res = read(fd, s + pos, n - pos);
> @@ -812,7 +814,8 @@ static void
> must_write(int fd, void *buf, size_t n)
> {
> char *s = buf;
> - ssize_t res, pos = 0;
> + ssize_t res;
> + size_t pos = 0;
>
> while (n > pos) {
> res = write(fd, s + pos, n - pos);
> Index: usr.sbin/syslogd/syslogd.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.117
> diff -u -p -r1.117 syslogd.c
> --- usr.sbin/syslogd/syslogd.c 22 Aug 2014 16:14:11 -0000 1.117
> +++ usr.sbin/syslogd/syslogd.c 22 Aug 2014 18:42:59 -0000
> @@ -868,10 +868,10 @@ fprintlog(struct filed *f, int flags, ch
>
> v = iov;
> if (f->f_type == F_WALL) {
> - if ((l = snprintf(greetings, sizeof(greetings),
> + l = snprintf(greetings, sizeof(greetings),
> "\r\n\7Message from syslogd@%s at %.24s ...\r\n",
> - f->f_prevhost, ctime(&now))) >= sizeof(greetings) ||
> - l == -1)
> + f->f_prevhost, ctime(&now));
> + if (l < 0 || (size_t)l >= sizeof(greetings))
> l = strlen(greetings);
> v->iov_base = greetings;
> v->iov_len = l;
> @@ -898,9 +898,9 @@ fprintlog(struct filed *f, int flags, ch
> v->iov_base = msg;
> v->iov_len = strlen(msg);
> } else if (f->f_prevcount > 1) {
> - if ((l = snprintf(repbuf, sizeof(repbuf),
> - "last message repeated %d times", f->f_prevcount)) >=
> - sizeof(repbuf) || l == -1)
> + l = snprintf(repbuf, sizeof(repbuf),
> + "last message repeated %d times", f->f_prevcount);
> + if (l < 0 || (size_t)l >= sizeof(repbuf))
> l = strlen(repbuf);
> v->iov_base = repbuf;
> v->iov_len = l;
> @@ -931,11 +931,12 @@ fprintlog(struct filed *f, int flags, ch
> fd = -1;
> break;
> }
> - if ((l = snprintf(line, sizeof(line), "<%d>%.15s %s%s%s",
> + l = snprintf(line, sizeof(line), "<%d>%.15s %s%s%s",
> f->f_prevpri, (char *)iov[0].iov_base,
> IncludeHostname ? LocalHostName : "",
> IncludeHostname ? " " : "",
> - (char *)iov[4].iov_base)) >= sizeof(line) || l == -1)
> + (char *)iov[4].iov_base);
> + if (l < 0 || (size_t)l >= sizeof(line))
> l = strlen(line);
> if (sendto(fd, line, l, 0,
> (struct sockaddr *)&f->f_un.f_forw.f_addr,
> @@ -1632,7 +1633,7 @@ cfline(char *line, char *prog)
> rb_len *= 1024;
>
> /* Copy buffer name */
> - for(i = 0; i < sizeof(f->f_un.f_mb.f_mname) - 1; i++) {
> + for(i = 0; (size_t)i < sizeof(f->f_un.f_mb.f_mname) - 1; i++) {
> if (!isalnum((unsigned char)q[i]))
> break;
> f->f_un.f_mb.f_mname[i] = q[i];
> Index: usr.sbin/syslogd/ttymsg.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/ttymsg.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 ttymsg.c
> --- usr.sbin/syslogd/ttymsg.c 27 Oct 2009 23:59:54 -0000 1.4
> +++ usr.sbin/syslogd/ttymsg.c 22 Aug 2014 18:42:59 -0000
> @@ -30,8 +30,11 @@
> * SUCH DAMAGE.
> */
>
> +#include <sys/param.h>
> +#include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/uio.h>
> +
> #include <signal.h>
> #include <fcntl.h>
> #include <dirent.h>
> @@ -41,7 +44,6 @@
> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> -#include <sys/stat.h>
>
> #include "syslogd.h"
>
> @@ -57,12 +59,13 @@ ttymsg(struct iovec *iov, int iovcnt, ch
> {
> static char device[MAXNAMLEN] = _PATH_DEV;
> static char errbuf[1024];
> - int cnt, fd, left, wret;
> + int cnt, fd, left;
> + ssize_t wret;
> struct iovec localiov[6];
> int forked = 0;
> sigset_t mask;
>
> - if (iovcnt > sizeof(localiov) / sizeof(localiov[0]))
> + if (iovcnt < 0 || (size_t)iovcnt > nitems(localiov))
> return ("too many iov's (change code in syslogd/ttymsg.c)");
>
> /*
> @@ -107,7 +110,7 @@ ttymsg(struct iovec *iov, int iovcnt, ch
> iovcnt * sizeof(struct iovec));
> iov = localiov;
> }
> - for (cnt = 0; wret >= iov->iov_len; ++cnt) {
> + for (cnt = 0; (size_t)wret >= iov->iov_len; ++cnt) {
> wret -= iov->iov_len;
> ++iov;
> --iovcnt;
