On Wed, 3 Nov 2010, Erik Faye-Lund wrote:
> On Wed, Nov 3, 2010 at 11:18 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
> > On Wed, Nov 3, 2010 at 10:11 PM, Pat Thoyts
> > <patthoyts@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >> Erik Faye-Lund <kusmabite@xxxxxxxxx> writes:
> >>
> >>>Here's hopefully the last iteration of this series. The previous version
> >>>only got a single complain about a typo in the subject of patch 14/15, so
> >>>it seems like most controversies have been settled.
> >>
> >> I pulled this win32-daemon branch into my msysgit build tree and built
> >> it. I get the following warnings:
> >>
> >> CC daemon.o
> >> daemon.c: In function 'service_loop':
> >> daemon.c:674: warning: dereferencing pointer 'ss.124' does break strict-aliasing rules
> >> daemon.c:676: warning: dereferencing pointer 'ss.124' does break strict-aliasing rules
> >> daemon.c:681: warning: dereferencing pointer 'ss.124' does break strict-aliasing rules
> >> daemon.c:919: note: initialized from here
> >> daemon.c:679: warning: dereferencing pointer 'sin_addr' does break strict-aliasing rules
> >> daemon.c:675: note: initialized from here
> >> daemon.c:691: warning: dereferencing pointer 'sin6_addr' does break strict-aliasing rules
> >> daemon.c:682: note: initialized from here
> >>
> >
> > Yeah, I'm aware of these. I thought those warnings were already
> > present in the Linux build, but checking again I see that that's not
> > the case. Need to investigate.
> >
>
> OK, it's the patch "daemon: use run-command api for async serving"
> that introduce the warning. But looking closer at the patch it doesn't
> seem the patch actually introduce the strict-aliasing violation, it's
> there already. The patch only seems to change the code enough for GCC
> to start realize there's a problem. Unless I'm misunderstanding
> something vital, that is.
>
> Anyway, here's a patch that makes it go away, I guess I'll squash it
> into the next round.
>
> diff --git a/daemon.c b/daemon.c
> index 6eee570..d636446 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -1159,14 +1159,13 @@ int main(int argc, char **argv)
> }
>
> if (inetd_mode || serve_mode) {
> - struct sockaddr_storage ss;
> - struct sockaddr *peer = (struct sockaddr *)&ss;
> - socklen_t slen = sizeof(ss);
> + struct sockaddr sa;
> + socklen_t slen = sizeof(sa);
>
> - if (getpeername(0, peer, &slen))
> - peer = NULL;
> -
> - return execute(peer);
> + if (getpeername(0, &sa, &slen))
> + return execute(NULL);
> + else
> + return execute(&sa);
> }
>
> if (detach) {
As you noticed later yourself, this is not right. You can get any kind of
sockaddr back from getpeername. I'm not sure if any spec mandates that a
"normal" sockaddr_in should fit into the base sockaddr, or if that just
happens by coincidence or sheer luck.
// Martin