From owner-svn-src-all@FreeBSD.ORG Tue Aug 10 15:40:22 2010 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5D71C106566C; Tue, 10 Aug 2010 15:40:22 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id F29888FC1A; Tue, 10 Aug 2010 15:40:21 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id o7AFaOPp075180; Tue, 10 Aug 2010 09:36:25 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Tue, 10 Aug 2010 09:36:56 -0600 (MDT) Message-Id: <20100810.093656.167578749323544001.imp@bsdimp.com> To: olli@fromme.com From: "M. Warner Losh" In-Reply-To: <201008101313.o7ADDhYE040900@haluter.fromme.com> References: <20100808.153608.1142818667055052395.imp@bsdimp.com> <201008101313.o7ADDhYE040900@haluter.fromme.com> X-Mailer: Mew version 6.3 on Emacs 22.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: src-committers@FreeBSD.org, jilles@stack.nl, svn-src-all@FreeBSD.org, olli@FreeBSD.org, svn-src-head@FreeBSD.org, des@des.no Subject: Re: svn commit: r211023 - head/usr.sbin/syslogd X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Aug 2010 15:40:22 -0000 In message: <201008101313.o7ADDhYE040900@haluter.fromme.com> Oliver Fromme writes: : M. Warner Losh wrote: : > The casting that syslogd does with struct sockaddrFOO is what triggers : > it. I'm not sure how to fix it, so there's about 6 or 8 programs in : > the tree that have WARNS lowered to 3 because of it. : : I've given it try, please see the patch below. : This is not really pretty, but it's a start. : It builds with WARNS=6 on all archs, including mips. : : What do you think? I have just one comment at the end about the removal of the u_int32_t casts. Otherwise, this looks really good. Thanks for taking the time to do this. : Best regards : Oliver : : --- syslogd.c.orig 2010-08-05 21:59:11.000000000 +0200 : +++ syslogd.c 2010-08-10 15:02:19.000000000 +0200 : @@ -175,7 +175,7 @@ : struct { : char f_hname[MAXHOSTNAMELEN]; : struct addrinfo *f_addr; : - : + in_port_t f_port; : } f_forw; /* forwarding address */ : char f_fname[MAXPATHLEN]; : struct { : @@ -330,8 +330,8 @@ : static void readklog(void); : static void reapchild(int); : static void usage(void); : -static int validate(struct sockaddr *, const char *); : -static void unmapped(struct sockaddr *); : +static int validate(struct sockaddr_storage *, const char *); : +static void unmapped(struct sockaddr_storage *); : static void wallmsg(struct filed *, struct iovec *, const int iovlen); : static int waitdaemon(int, int, int); : static void timedout(int); : @@ -652,8 +652,8 @@ : if (l > 0) { : line[l] = '\0'; : hname = cvthname((struct sockaddr *)&frominet); : - unmapped((struct sockaddr *)&frominet); : - if (validate((struct sockaddr *)&frominet, hname)) : + unmapped(&frominet); : + if (validate(&frominet, hname)) : printline(hname, line, RemoteAddDate ? ADDDATE : 0); : } else if (l < 0 && errno != EINTR) : logerror("recvfrom inet"); : @@ -678,17 +678,17 @@ : } : : static void : -unmapped(struct sockaddr *sa) : +unmapped(struct sockaddr_storage *ss) : { : struct sockaddr_in6 *sin6; : struct sockaddr_in sin4; : : - if (sa->sa_family != AF_INET6) : + if (ss->ss_family != AF_INET6) : return; : - if (sa->sa_len != sizeof(struct sockaddr_in6) || : - sizeof(sin4) > sa->sa_len) : + if (ss->ss_len != sizeof(struct sockaddr_in6) || : + sizeof(sin4) > ss->ss_len) : return; : - sin6 = (struct sockaddr_in6 *)sa; : + sin6 = (struct sockaddr_in6 *)ss; : if (!IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) : return; : : @@ -699,7 +699,7 @@ : sizeof(sin4.sin_addr)); : sin4.sin_port = sin6->sin6_port; : : - memcpy(sa, &sin4, sin4.sin_len); : + memcpy(ss, &sin4, sin4.sin_len); : } : : static void : @@ -1186,8 +1186,7 @@ : break; : : case F_FORW: : - port = (int)ntohs(((struct sockaddr_in *) : - (f->f_un.f_forw.f_addr->ai_addr))->sin_port); : + port = f->f_un.f_forw.f_port; : if (port != 514) { : dprintf(" %s:%d\n", f->f_un.f_forw.f_hname, port); : } else { : @@ -1711,8 +1710,7 @@ : break; : : case F_FORW: : - port = (int)ntohs(((struct sockaddr_in *) : - (f->f_un.f_forw.f_addr->ai_addr))->sin_port); : + port = f->f_un.f_forw.f_port; : if (port != 514) { : printf("%s:%d", : f->f_un.f_forw.f_hname, port); : @@ -1767,6 +1765,7 @@ : cfline(const char *line, struct filed *f, const char *prog, const char *host) : { : struct addrinfo hints, *res; : + struct servent *srvent; : int error, i, pri, syncfile; : const char *p, *q; : char *bp; : @@ -1954,6 +1953,12 @@ : break; : } : f->f_un.f_forw.f_addr = res; : + srvent = getservbyname(p ? p : "syslog", NULL); : + if (srvent != NULL) : + f->f_un.f_forw.f_port = srvent->s_port; : + else : + /* Fallback, shouldn't happen. */ : + f->f_un.f_forw.f_port = 514; : f->f_type = F_FORW; : break; : : @@ -2340,7 +2345,7 @@ : * Validate that the remote peer has permission to log to us. : */ : static int : -validate(struct sockaddr *sa, const char *hname) : +validate(struct sockaddr_storage *ss, const char *hname) : { : int i; : size_t l1, l2; : @@ -2369,8 +2374,8 @@ : strlcat(name, ".", sizeof name); : strlcat(name, LocalDomain, sizeof name); : } : - if (getnameinfo(sa, sa->sa_len, ip, sizeof ip, port, sizeof port, : - NI_NUMERICHOST | NI_NUMERICSERV) != 0) : + if (getnameinfo((struct sockaddr *)ss, ss->ss_len, ip, sizeof ip, port, : + sizeof port, NI_NUMERICHOST | NI_NUMERICSERV) != 0) : return (0); /* for safety, should not occur */ : dprintf("validate: dgram from IP %s, port %s, name %s;\n", : ip, port, name); : @@ -2384,12 +2389,12 @@ : } : : if (ap->isnumeric) { : - if (ap->a_addr.ss_family != sa->sa_family) { : + if (ap->a_addr.ss_family != ss->ss_family) { : dprintf("rejected in rule %d due to address family mismatch.\n", i); : continue; : } : if (ap->a_addr.ss_family == AF_INET) { : - sin4 = (struct sockaddr_in *)sa; : + sin4 = (struct sockaddr_in *)ss; : a4p = (struct sockaddr_in *)&ap->a_addr; : m4p = (struct sockaddr_in *)&ap->a_mask; : if ((sin4->sin_addr.s_addr & m4p->sin_addr.s_addr) : @@ -2400,7 +2405,7 @@ : } : #ifdef INET6 : else if (ap->a_addr.ss_family == AF_INET6) { : - sin6 = (struct sockaddr_in6 *)sa; : + sin6 = (struct sockaddr_in6 *)ss; : a6p = (struct sockaddr_in6 *)&ap->a_addr; : m6p = (struct sockaddr_in6 *)&ap->a_mask; : if (a6p->sin6_scope_id != 0 && : @@ -2409,9 +2414,9 @@ : continue; : } : reject = 0; : - for (j = 0; j < 16; j += 4) { : - if ((*(u_int32_t *)&sin6->sin6_addr.s6_addr[j] & *(u_int32_t *)&m6p->sin6_addr.s6_addr[j]) : - != *(u_int32_t *)&a6p->sin6_addr.s6_addr[j]) { : + for (j = 0; j < 16; j++) { : + if ((sin6->sin6_addr.s6_addr[j] & m6p->sin6_addr.s6_addr[j]) : + != a6p->sin6_addr.s6_addr[j]) { This code looks better, but why have the casts been removed? I take it they aren't necessary? : ++reject; : break; : } : : Warner