This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: Incomplete patch to fix build with top-of-tree GCC
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Paul Eggert <eggert at cs dot ucla dot edu>, Joseph Myers <joseph at codesourcery dot com>, Steve Ellcey <sellcey at imgtec dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Sat, 16 May 2015 00:13:24 -0400
- Subject: Re: Incomplete patch to fix build with top-of-tree GCC
- Authentication-results: sourceware.org; auth=none
- References: <1431708170 dot 16668 dot 8 dot camel at ubuntu-sellcey> <alpine dot DEB dot 2 dot 10 dot 1505151645070 dot 21212 at digraph dot polyomino dot org dot uk> <55565718 dot 6070107 at cs dot ucla dot edu>
On 05/15/2015 04:29 PM, Paul Eggert wrote:
> On 05/15/2015 09:47 AM, Joseph Myers wrote:
>> The first question is:*is* it OK to ignore the warnings?
> I looked just at the first hunk of the patch, and it appears to me
> that it's not OK to ignore its warnings. The type punning in the
> first hunk appears to violate the C standard; although I don't know
> whether GCC is generating "incorrect" code as a result, instead of
> silencing the warning how about fixing the code so that it doesn't
> have those funky casts? Something like the attached (untested) patch,
> say.
Yes please.
We should be fixing this everywhere we see obvious C strict-aliasing violations.
Your changes are mechanical and look good to me.
Cheers,
Carlos.
> diff --git a/inet/rcmd.c b/inet/rcmd.c
> index acacaa0..bb5b0aa 100644
> --- a/inet/rcmd.c
> +++ b/inet/rcmd.c
> @@ -374,7 +374,11 @@ rresvport_af(alport, family)
> int *alport;
> sa_family_t family;
> {
> - struct sockaddr_storage ss;
> + union {
> + struct sockaddr generic;
> + struct sockaddr_in in;
> + struct sockaddr_in6 in6;
> + } ss;
> int s;
> size_t len;
> uint16_t *sport;
> @@ -382,11 +386,11 @@ rresvport_af(alport, family)
> switch(family){
> case AF_INET:
> len = sizeof(struct sockaddr_in);
> - sport = &((struct sockaddr_in *)&ss)->sin_port;
> + sport = &ss.in.sin_port;
> break;
> case AF_INET6:
> len = sizeof(struct sockaddr_in6);
> - sport = &((struct sockaddr_in6 *)&ss)->sin6_port;
> + sport = &ss.in6.sin6_port;
> break;
> default:
> __set_errno (EAFNOSUPPORT);
> @@ -398,9 +402,9 @@ rresvport_af(alport, family)
>
> memset (&ss, '\0', sizeof(ss));
> #ifdef SALEN
> - ss.__ss_len = len;
> + ss.generic.__ss_len = len;
> #endif
> - ss.ss_family = family;
> + ss.generic.ss_family = family;
>
> /* Ignore invalid values. */
> if (*alport < IPPORT_RESERVED / 2)
> @@ -411,7 +415,7 @@ rresvport_af(alport, family)
> int start = *alport;
> do {
> *sport = htons((uint16_t) *alport);
> - if (__bind(s, (struct sockaddr *)&ss, len) >= 0)
> + if (__bind(s, &ss.generic, len) >= 0)
> return s;
> if (errno != EADDRINUSE) {
> (void)__close(s);