In message: <201008101313.o7addhye040...@haluter.fromme.com>
Oliver Fromme <o...@fromme.com> 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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
