On Thu, Mar 08, 2007 at 10:37:11AM -0500, Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
> > On Thu, Mar 08, 2007 at 10:10:28AM -0500, Tom Lane wrote:
> >> Also, isn't the WSACleanup() in freePGconn in the wrong place?  Seems
> >> like it shouldn't be done until after we've closed the socket.  I'd
> >> be inclined to put it at the bottom of the routine.
> 
> > Certainly looks wrong. It's interesting how this could have worked
> > *before*.
> 
> Because the calls in DllMain covered us (ie, the WSA usage count never
> got to be less than one).  If we remove them, we'd better get this pair
> right.

But those calls weren't even compiled in when building using mingw,
which is what the majority of our users have been using lately, I think.
(Since that's what we ship in the binary package)


> One thing that bothers me a bit is that if we just move the call to the
> bottom, then freePGconn won't do it at all if passed a NULL pointer.
> Now (assuming a non-broken app) the only way that can happen is if
> makeEmptyPGconn runs out of memory.  If the client gets back a null
> pointer from a connection attempt, it's probably a 50-50 proposition
> whether it will think it ought to do PQfinish with it.  So it'd be good
> if we could keep the usage count straight either way.  I propose the
> invariant "a WSA usage count is associated with a non-null PGconn
> structure".  That would mean that doing WSACleanup only at the bottom
> of freePGconn is correct, but also that makeEmptyPGconn needs to do
> WSACleanup in its (two) failure paths.

I'm honestly unsure wether we need to bother with it, but yeah, that
will likely be "more correct". 
(Except one of the error paths in makeEmptyPGconn is already covered,
since it calls freePGconn, which does the WSACleanup)

//Magnus

Index: bcc32.mak
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/bcc32.mak,v
retrieving revision 1.26
diff -c -r1.26 bcc32.mak
*** bcc32.mak   11 Jan 2007 02:42:31 -0000      1.26
--- bcc32.mak   8 Mar 2007 15:23:17 -0000
***************
*** 93,99 ****
        [EMAIL PROTECTED] "$(INTDIR)\fe-secure.obj"
        [EMAIL PROTECTED] "$(INTDIR)\pqexpbuffer.obj"
        [EMAIL PROTECTED] "$(INTDIR)\pqsignal.obj"
-       [EMAIL PROTECTED] "$(OUTDIR)\libpqdll.obj"
        [EMAIL PROTECTED] "$(OUTDIR)\win32.obj"
        [EMAIL PROTECTED] "$(INTDIR)\wchar.obj"
        [EMAIL PROTECTED] "$(INTDIR)\encnames.obj"
--- 93,98 ----
***************
*** 155,168 ****
  
  LINK32=ilink32.exe
  LINK32_FLAGS = -Gn -L$(BCB)\lib;$(INTDIR); -x -Tpd -v
- LINK32_OBJS= "$(INTDIR)\libpqdll.obj"
  
  # @<< is a Response file, http://www.opussoftware.com/tutorial/TutMakefile.htm
  
! "$(OUTDIR)\blibpq.dll": "$(OUTDIR)\blibpq.lib" $(LINK32_OBJS) 
"$(INTDIR)\libpq.res" blibpqdll.def 
        $(LINK32) @<<
        $(LINK32_FLAGS) +
!       c0d32.obj $(LINK32_OBJS), +
        $@,, +
        "$(OUTDIR)\blibpq.lib" import32.lib cw32mt.lib, +
        blibpqdll.def,"$(INTDIR)\libpq.res"
--- 154,166 ----
  
  LINK32=ilink32.exe
  LINK32_FLAGS = -Gn -L$(BCB)\lib;$(INTDIR); -x -Tpd -v
  
  # @<< is a Response file, http://www.opussoftware.com/tutorial/TutMakefile.htm
  
! "$(OUTDIR)\blibpq.dll": "$(OUTDIR)\blibpq.lib" "$(INTDIR)\libpq.res" 
blibpqdll.def 
        $(LINK32) @<<
        $(LINK32_FLAGS) +
!       c0d32.obj , +
        $@,, +
        "$(OUTDIR)\blibpq.lib" import32.lib cw32mt.lib, +
        blibpqdll.def,"$(INTDIR)\libpq.res"
Index: fe-connect.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.344
diff -c -r1.344 fe-connect.c
*** fe-connect.c        20 Feb 2007 15:20:51 -0000      1.344
--- fe-connect.c        8 Mar 2007 15:42:59 -0000
***************
*** 1840,1848 ****
  #ifdef WIN32
  
        /*
!        * Make sure socket support is up and running. Even though this is done 
in
!        * libpqdll.c, that is only for MSVC and BCC builds and doesn't work for
!        * static builds at all, so we have to do it in the main code too.
         */
        WSADATA         wsaData;
  
--- 1840,1846 ----
  #ifdef WIN32
  
        /*
!        * Make sure socket support is up and running.
         */
        WSADATA         wsaData;
  
***************
*** 1853,1859 ****
--- 1851,1862 ----
  
        conn = (PGconn *) malloc(sizeof(PGconn));
        if (conn == NULL)
+       {
+ #ifdef WIN32
+               WSACleanup();
+ #endif
                return conn;
+       }
  
        /* Zero all pointers and booleans */
        MemSet(conn, 0, sizeof(PGconn));
***************
*** 1918,1927 ****
        PGnotify   *notify;
        pgParameterStatus *pstatus;
  
- #ifdef WIN32
-       WSACleanup();
- #endif
- 
        if (!conn)
                return;
  
--- 1921,1926 ----
***************
*** 1986,1991 ****
--- 1985,1994 ----
        termPQExpBuffer(&conn->errorMessage);
        termPQExpBuffer(&conn->workBuffer);
        free(conn);
+
+ #ifdef WIN32
+       WSACleanup();
+ #endif
  }

  /*
Index: win32.mak
===================================================================
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/win32.mak,v
retrieving revision 1.43
diff -c -r1.43 win32.mak
*** win32.mak   11 Jan 2007 02:42:31 -0000      1.43
--- win32.mak   8 Mar 2007 15:22:29 -0000
***************
*** 63,69 ****
        [EMAIL PROTECTED] "$(INTDIR)\fe-secure.obj"
        [EMAIL PROTECTED] "$(INTDIR)\pqexpbuffer.obj"
        [EMAIL PROTECTED] "$(INTDIR)\pqsignal.obj"
-       [EMAIL PROTECTED] "$(OUTDIR)\libpqdll.obj"
        [EMAIL PROTECTED] "$(OUTDIR)\win32.obj"
        [EMAIL PROTECTED] "$(INTDIR)\wchar.obj"
        [EMAIL PROTECTED] "$(INTDIR)\encnames.obj"
--- 63,68 ----
***************
*** 143,149 ****
   /pdb:"$(OUTDIR)\libpqdll.pdb" /machine:I386 
/out:"$(OUTDIR)\$(OUTFILENAME).dll"\
   /implib:"$(OUTDIR)\$(OUTFILENAME)dll.lib"  /def:$(OUTFILENAME)dll.def
  LINK32_OBJS= \
-       "$(INTDIR)\libpqdll.obj" \
        "$(OUTDIR)\$(OUTFILENAME).lib" \
        "$(OUTDIR)\libpq.res"

--- 142,147 ----
***************
*** 159,165 ****
        $(RSC) $(RSC_PROJ) libpq.rc


! "$(OUTDIR)\$(OUTFILENAME).dll" : "$(OUTDIR)" "$(OUTDIR)\libpqdll.obj" 
"$(INTDIR)\libpqdll.obj" "$(INTDIR)\libpq.res"
        $(LINK32) @<<
        $(LINK32_FLAGS) $(LINK32_OBJS)
  <<
--- 157,163 ----
        $(RSC) $(RSC_PROJ) libpq.rc


! "$(OUTDIR)\$(OUTFILENAME).dll" : "$(OUTDIR)" "$(INTDIR)\libpq.res"
        $(LINK32) @<<
        $(LINK32_FLAGS) $(LINK32_OBJS)
  <<
---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
       subscribe-nomail command to [EMAIL PROTECTED] so that your
       message can get through to the mailing list cleanly

Reply via email to