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';
