Thread: Re: [HACKERS] Proposed patch to getaddrinfo.c to support
It, or some related patch appears to have broken the build on buildfarm member snake. I haven't had time to investigate. /D -----Original Message----- From: "Bruce Momjian"<pgman@candle.pha.pa.us> Sent: 25/08/05 01:14:54 To: "Tom Lane"<tgl@sss.pgh.pa.us> Cc: "Andrew Dunstan"<andrew@dunslane.net>, "Chuck McDevitt"<cmcdevitt@greenplum.com>, "pgsql-patches@postgresql.org"<pgsql-patches@postgresql.org>,"PostgreSQL-development"<pgsql-hackers@postgresql.org> Subject: Re: [HACKERS] [PATCHES] Proposed patch to getaddrinfo.c to support Does this fix IPv6 on Win32? --------------------------------------------------------------------------- Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> Context diff, please, diff -c. > > > It needed dos2unix and pgindent as well. Here's a cleaned patch. > > Thanks to Chuck for doing this work. > > Applied, thanks. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 ---------------------------(end of broadcast)--------------------------- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to majordomo@postgresql.org so that your message can get through to the mailing list cleanly -----Unmodified Original Message----- Does this fix IPv6 on Win32? --------------------------------------------------------------------------- Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > >> Context diff, please, diff -c. > > > It needed dos2unix and pgindent as well. Here's a cleaned patch. > > Thanks to Chuck for doing this work. > > Applied, thanks. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 ---------------------------(end of broadcast)--------------------------- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to majordomo@postgresql.org so that your message can get through to the mailing list cleanly
Dave Page wrote:
> It, or some related patch appears to have broken the build on buildfarm member snake.
>
> I haven't had time to investigate.
>
> /D
Atached patch fixes it and also adds proper gai_strerror for windows.
(It's patch against CVS *after* Chucks patch was aplied)
--
Regards
Petr Jelinek (PJMODOS)
Index: src/include/getaddrinfo.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/getaddrinfo.h,v
retrieving revision 1.15
diff -c -r1.15 getaddrinfo.h
*** src/include/getaddrinfo.h 27 Jul 2005 12:44:10 -0000 1.15
--- src/include/getaddrinfo.h 25 Aug 2005 09:39:09 -0000
***************
*** 30,35 ****
--- 30,46 ----
/* Various macros that ought to be in <netdb.h>, but might not be */
+ #ifdef WIN32
+ #define EAI_AGAIN WSATRY_AGAIN
+ #define EAI_BADFLAGS WSAEINVAL
+ #define EAI_FAIL WSANO_RECOVERY
+ #define EAI_FAMILY WSAEAFNOSUPPORT
+ #define EAI_MEMORY WSA_NOT_ENOUGH_MEMORY
+ #define EAI_NODATA WSANO_DATA
+ #define EAI_NONAME WSAHOST_NOT_FOUND
+ #define EAI_SERVICE WSATYPE_NOT_FOUND
+ #define EAI_SOCKTYPE WSAESOCKTNOSUPPORT
+ #else
#ifndef EAI_FAIL
#define EAI_BADFLAGS (-1)
#define EAI_NONAME (-2)
***************
*** 40,46 ****
#define EAI_SERVICE (-8)
#define EAI_MEMORY (-10)
#define EAI_SYSTEM (-11)
! #endif
#ifndef AI_PASSIVE
#define AI_PASSIVE 0x0001
--- 51,58 ----
#define EAI_SERVICE (-8)
#define EAI_MEMORY (-10)
#define EAI_SYSTEM (-11)
! #endif /* !EAI_FAIL */
! #endif /* !WIN32 */
#ifndef AI_PASSIVE
#define AI_PASSIVE 0x0001
Index: src/port/getaddrinfo.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/port/getaddrinfo.c,v
retrieving revision 1.18
diff -c -r1.18 getaddrinfo.c
*** src/port/getaddrinfo.c 24 Aug 2005 22:13:23 -0000 1.18
--- src/port/getaddrinfo.c 25 Aug 2005 09:39:25 -0000
***************
*** 104,110 ****
/* We found a dll, so now get the addresses of the routines */
getaddrinfo_ptr = GetProcAddress(hLibrary, "getaddrinfo");
! freeaddrinfo_ptr = GetProcAddress(hLibrary, "freeaddrinfo");
getnameinfo_ptr = GetProcAddress(hLibrary, "getnameinfo");
/*
--- 104,110 ----
/* We found a dll, so now get the addresses of the routines */
getaddrinfo_ptr = GetProcAddress(hLibrary, "getaddrinfo");
! freeaddrinfo_ptr = (freeaddrinfo_ptr_t)GetProcAddress(hLibrary, "freeaddrinfo");
getnameinfo_ptr = GetProcAddress(hLibrary, "getnameinfo");
/*
***************
*** 277,283 ****
*/
if (haveNativeWindowsIPv6routines())
{
! (*freeaddrinfo_ptr) (node, service, hintp, res);
return;
}
#endif
--- 277,283 ----
*/
if (haveNativeWindowsIPv6routines())
{
! (*freeaddrinfo_ptr) (res);
return;
}
#endif
***************
*** 292,298 ****
const char *
gai_strerror(int errcode)
{
! #ifdef HAVE_HSTRERROR
int hcode;
switch (errcode)
--- 292,298 ----
const char *
gai_strerror(int errcode)
{
! #ifdef HAVE_HSTRERROR
int hcode;
switch (errcode)
***************
*** 318,323 ****
--- 318,336 ----
return "Unknown host";
case EAI_AGAIN:
return "Host name lookup failure";
+ /* Errors below are probably WIN32 only */
+ case EAI_BADFLAGS:
+ return "Invalid argument";
+ case EAI_FAMILY:
+ return "Address family not supported";
+ case EAI_MEMORY:
+ return "Not enough memory";
+ case EAI_NODATA:
+ return "No host data of that type was found";
+ case EAI_SERVICE:
+ return "Class type not found";
+ case EAI_SOCKTYPE:
+ return "Socket type not supported";
case EAI_FAIL:
default:
return "Unknown server error";
Petr Jelinek wrote: > Dave Page wrote: > >> It, or some related patch appears to have broken the build on >> buildfarm member snake. >> >> I haven't had time to investigate. >> >> /D > > > Atached patch fixes it and also adds proper gai_strerror for windows. > (It's patch against CVS *after* Chucks patch was aplied) > I thought this had been tested. I should have tested it myself. Apologies. Anyway, with Petr's extra patch I get a clean build, but "make check" fails with a postmaster bind failure and a pgsql failure, both with and without IPv6 installed, on my Xp-PRO SP1 box. When IPv6 is installed it complains about an unknown family 23 (which is Windows-speak for AF_INET6). So, not quite there yet. I'm out of action for pretty much the rest of today and tomorrow, so won't be doing more testing for a while. cheers andrew
Petr Jelinek <pjmodos@seznam.cz> writes:
> Dave Page wrote:
>> It, or some related patch appears to have broken the build on buildfarm member snake.
> Atached patch fixes it and also adds proper gai_strerror for windows.
Applied. I had to #ifdef the gai_strerror additions to avoid breakage
on my own machine.
regards, tom lane
Andrew Dunstan wrote: > > I thought this had been tested. I should have tested it myself. Apologies. > Right, I thought I tested it, well maybe it was my version dunno but surely it was my mistake. > Anyway, with Petr's extra patch I get a clean build, but "make check" > fails with a postmaster bind failure and a pgsql failure, both with and > without IPv6 installed, on my Xp-PRO SP1 box. When IPv6 is installed it > complains about an unknown family 23 (which is Windows-speak for AF_INET6). > > So, not quite there yet. [I did make check only in W2K because I don't have direct access to XP machine now] No thats not windows error thats postgres error (look at pqcomm.c), which means HAVE_IPV6 is not defined. I think it should be made that HAVE_IPV6 and HAVE_STRUCT_ADDRINFO is always defined under windows (and also #include <ws2tcpip.h> in getaddrinfo.h otherwise it won't build) but I am not familiar with build system so somebody else will have to do it (I am not familiar with whole configure thingy at all). With those changes it should finally work. -- Regards Petr Jelinek (PJMODOS)
Petr Jelinek <pjmodos@seznam.cz> writes: > Andrew Dunstan wrote: >> So, not quite there yet. > [I did make check only in W2K because I don't have direct access to XP > machine now] > No thats not windows error thats postgres error (look at pqcomm.c), > which means HAVE_IPV6 is not defined. Possibly, but that's apparently not the only problem. I'm looking at the first buildfarm result with this patch, http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=loris&dt=2005-08-25%2018:56:02 The interesting part is the postmaster log at the bottom: LOG: could not bind IPv4 socket: No error HINT: Is another postmaster already running on port 55678? If not, wait a few seconds and retry. WARNING: could not create listen socket for "localhost" FATAL: could not create any TCP/IP sockets Apparently, access to IPv4 sockets isn't working either (and the "No error" isn't very helpful; would seem we're not reading the right status value). regards, tom lane
Tom Lane wrote: > Possibly, but that's apparently not the only problem. I'm looking at > the first buildfarm result with this patch, > http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=loris&dt=2005-08-25%2018:56:02 > > The interesting part is the postmaster log at the bottom: > > LOG: could not bind IPv4 socket: No error > HINT: Is another postmaster already running on port 55678? If not, wait a few seconds and retry. > WARNING: could not create listen socket for "localhost" > FATAL: could not create any TCP/IP sockets > > Apparently, access to IPv4 sockets isn't working either (and the "No > error" isn't very helpful; would seem we're not reading the right > status value). /me kicks brother out of winXP machine to see whats going on Yep those changes proposed in my previous email fixes IPv4 too. LOG: database system was shut down at 2005-08-26 00:05:51 [removed unreadable chars :)] LOG: checkpoint record is at 0/390CE0 LOG: redo record is at 0/390CE0; undo record is at 0/0; shutdown TRUE LOG: next transaction ID: 562; next OID: 10791 LOG: next MultiXactId: 1; next MultiXactOffset: 0 LOG: database system is ready LOG: transaction ID wrap limit is 2147484144, limited by database "postgres" Looks ok, i don't know what IPv4 has to do with all of this though. -- Regards Petr Jelinek (PJMODOS)
Petr Jelinek <pjmodos@seznam.cz> writes:
> Yep those changes proposed in my previous email fixes IPv4 too.
Apparently not on loris (unless there was another patch that I missed).
Maybe something to do with a different version of Windows?
regards, tom lane
Tom Lane wrote:
>Petr Jelinek <pjmodos@seznam.cz> writes:
>
>
>>Yep those changes proposed in my previous email fixes IPv4 too.
>>
>>
>
>Apparently not on loris (unless there was another patch that I missed).
>Maybe something to do with a different version of Windows?
>
>
>
>
I suspected we'd forgotten something.
The attached small patch appears to be what's required (at least on
loris). "make check" failed but not for any apparent ipv6 reason. More
importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO.
cheers
andrew
Index: src/include/port/win32/sys/socket.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/port/win32/sys/socket.h,v
retrieving revision 1.2
diff -c -r1.2 socket.h
*** src/include/port/win32/sys/socket.h 4 Aug 2003 00:43:32 -0000 1.2
--- src/include/port/win32/sys/socket.h 25 Aug 2005 22:55:39 -0000
***************
*** 5,10 ****
--- 5,11 ----
*
* Note: Don't include <wingdi.h> directly. It causes compile errors.
*/
+ #include <ws2tcpip.h>
#include <winsock2.h>
#undef ERROR
#undef small
***************
*** 13,16 ****
--- 14,24 ----
#ifdef PGERROR
#define ERROR PGERROR
+ /*
+ * we can't use the windows gai_strerror{AW} functions because
+ * they are defined inline in the MS header files. So we'll use our
+ * own
+ */
+ #undef gai_strerror
+
#endif
Andrew Dunstan <andrew@dunslane.net> writes:
> I suspected we'd forgotten something.
> The attached small patch appears to be what's required (at least on
> loris). "make check" failed but not for any apparent ipv6 reason. More
> importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO.
Applied. One step at a time ;-)
regards, tom lane
Andrew Dunstan wrote: > I suspected we'd forgotten something. > > The attached small patch appears to be what's required (at least on > loris). "make check" failed but not for any apparent ipv6 reason. More > importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO. Well this is what I ment with those proposed changes - I haven't sent patch, just said whats needed - always define HAVE_IPV6 and HAVE_STRUCT_ADDRINFO and include ws2tcpip.h, if you do just that include like your patch did, you'll break building on W2k (and like I said I don't know how to make HAVE_IPV6 and HAVE_STRUCT_ADDRINFO always defined under windows because I am not familiar with configure and thats why I haven't sent patch). -- Regards Petr Jelinek (PJMODOS)
Petr Jelinek wrote: > Andrew Dunstan wrote: > >> I suspected we'd forgotten something. >> >> The attached small patch appears to be what's required (at least on >> loris). "make check" failed but not for any apparent ipv6 reason. >> More importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO. > > > Well this is what I ment with those proposed changes - I haven't sent > patch, just said whats needed - always define HAVE_IPV6 and > HAVE_STRUCT_ADDRINFO and include ws2tcpip.h, if you do just that > include like your patch did, you'll break building on W2k (and like I > said I don't know how to make HAVE_IPV6 and HAVE_STRUCT_ADDRINFO > always defined under windows because I am not familiar with configure > and thats why I haven't sent patch). > Really? Please don't assert it, test it and tell us what the error is. I find it highly unlikely that it will break building on w2k, and only slightly less unlikely that it will break running on w2k. But I want to see the evidence (make error or error from runtime log). The patch I sent should be exactly what is required to have HAVE_IPV6 and HAVE_STRUCT_ADDRINFO defined on windows. That should be true regardless of which windows you are building on - the headers should be the same. cheers andrew
Andrew Dunstan wrote: > > The patch I sent should be exactly what is required to have HAVE_IPV6 > and HAVE_STRUCT_ADDRINFO defined on windows. That should be true > regardless of which windows you are building on - the headers should be > the same. Oh, if that include makes HAVE_IPV6 defined than it should be ok, I guess I just misunderstood your mail about your changes to HAVE_IPV6 check in configure under windows. Sorry for misinformation. [making...] Yes it actually builds, make check has nine failures for me but that has nothing to do with IPv6 (looks like postgres doesn't like my locale because with initdb --no-locale it passes without prob). So I hope IPv6 episode is finally over :) -- Regards Petr Jelinek (PJMODOS)
Petr Jelinek wrote:
>
> So I hope IPv6 episode is finally over :)
>
Almost :-)
The initdb code is failing because we didn't call WSAStartup() - this
just took me ages to track down. All the rest works.
The attached patch seems to do the trick.
cheers
andrew
Index: initdb.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/bin/initdb/initdb.c,v
retrieving revision 1.96
diff -c -r1.96 initdb.c
*** initdb.c 25 Aug 2005 02:22:59 -0000 1.96
--- initdb.c 26 Aug 2005 22:28:48 -0000
***************
*** 1221,1226 ****
--- 1221,1237 ----
{
struct addrinfo *gai_result;
struct addrinfo hints;
+ int err = 0;
+
+ #ifdef WIN32
+ /* need to call this before calling getaddrinfo */
+
+ WSADATA wsaData;
+
+ err = WSAStartup(MAKEWORD(2,2),&wsaData);
+
+ #endif;
+
/* for best results, this code should match parse_hba() */
hints.ai_flags = AI_NUMERICHOST;
***************
*** 1232,1238 ****
hints.ai_addr = NULL;
hints.ai_next = NULL;
! if (getaddrinfo("::1", NULL, &hints, &gai_result) != 0)
conflines = replace_token(conflines,
"host all all ::1",
"#host all all ::1");
--- 1243,1249 ----
hints.ai_addr = NULL;
hints.ai_next = NULL;
! if ( err != 0 || getaddrinfo("::1", NULL, &hints, &gai_result) != 0)
conflines = replace_token(conflines,
"host all all ::1",
"#host all all ::1");
Andrew Dunstan <andrew@dunslane.net> writes:
> The initdb code is failing because we didn't call WSAStartup() - this
> just took me ages to track down. All the rest works.
> The attached patch seems to do the trick.
Applied.
regards, tom lane