On Wed, Nov 3, 2010 at 11:58 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> 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. > I also of course need to update "daemon: get remote host address from root-process" as well, as it introduces a new such code-path (which is actually the one complained about here). And I guess I should use sockaddr_in instead of sockaddr. But my luck stops there. The resulting git-daemon.exe leaves me with a very bizarre error: error: unable to make a socket file descriptor: Bad file descriptor fatal: accept returned: Bad file descriptor This is triggered by the call to accept() in mingw_accept returning -1. What is even stranger is that if I change the code at the error-point like this - struct sockaddr_in sa; + struct sockaddr_in sa[2]; socklen_t salen = sizeof(sa); - int incoming = accept(pfd[i].fd, (struct sockaddr *)&sa, &salen); + int incoming = accept(pfd[i].fd, (struct sockaddr *)sa, &salen); the error goes away. Similarly, if I change mingw_accept's call to Winsock's accept(), like this: - SOCKET s2 = accept(s1, sa, sz); + SOCKET s2 = accept(s1, sa, NULL); So it seems accept() somehow reacts to the value of the variable pointed at by sz, which is 16. Strange, huh? Perhaps it isn't -- the sockaddr_storage change seems to have been introduced for IPv6 reasons. I'm trying to connect over IPv6, and IPv6 has a new sockaddr_in6 struct. So yeah. Stuffing all of sockaddr, sockaddr_in and sockaddr_in6 (when built with IPv6 support) in a union and passing that around instead does seem to fix the issue completely. I don't find it very elegant, but some google-searches on the issue seems to reveal that this is the only way of getting rid of this. Any other suggestions, people? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html