On Tue, Nov 27, 2018 at 09:03:15AM +0100, Theo Buehler wrote: > On Sun, Nov 25, 2018 at 12:32:23AM +0100, Jan Klemkow wrote: > > This diff fixes some -Wsign-compare compiler warnings in ftpd(8) by > > using the right types for 'i' and 'len'. One warning is left, but I > > don't see that it's fixable without suppressing the warning by a cast of > > len to size_t. And casting might be controversial in this case?! > > The diff looks correct to me. If anyone wants to commit > > ok tb > > > /usr/src/libexec/ftpd/ftpd.c:2781:11: warning: comparison of integers of > > different signs: 'int' and > > 'unsigned long' [-Wsign-compare] > > if (len >= sizeof(buf) || len == -1) { > > ~~~ ^ ~~~~~~~~~~~ > > This test is the wrong way around: compare CAVEATS in snprintf(3).
I missed that. The following diff fixes that, but doesn't include a cast. > There's a ton of unchecked snprintfs in this code. Did you take a look > at those? Yes, I noticed that. I may change that in a later diff. Thanks, Jan Index: ftpd.c =================================================================== RCS file: /cvs/src/libexec/ftpd/ftpd.c,v retrieving revision 1.223 diff -u -p -r1.223 ftpd.c --- ftpd.c 3 Sep 2016 15:00:48 -0000 1.223 +++ ftpd.c 6 Dec 2018 18:19:21 -0000 @@ -390,10 +390,10 @@ main(int argc, char *argv[]) endpwent(); if (daemon_mode) { - int *fds, i, fd; + int *fds, fd; struct pollfd *pfds; struct addrinfo hints, *res, *res0; - nfds_t n; + nfds_t n, i; /* * Detach from parent. @@ -1809,8 +1809,8 @@ statcmd(void) ispassive++; goto printaddr; } else if (usedefault == 0) { - size_t alen; - int af, i; + size_t alen, i; + int af; su = (union sockunion *)&data_dest; printaddr: @@ -2545,7 +2545,8 @@ guniquefd(char *local, char **nam) { static char new[PATH_MAX]; struct stat st; - int count, len, fd; + size_t len; + int count, fd; char *cp; cp = strrchr(local, '/'); @@ -2777,7 +2778,7 @@ logxfer(char *name, off_t size, time_t s ((guest) ? "*" : pw->pw_name), dhostname); free(vpw); - if (len >= sizeof(buf) || len == -1) { + if (len == -1 || len >= sizeof(buf)) { if ((len = strlen(buf)) == 0) return; /* should not happen */ buf[len - 1] = '\n';