Re: Postgresql on win32

Thanks.  Applied.

[ Charset ISO-8859-1 unsupported, converting... ]
> Hello!
> 
> Here is a patch to make the current snapshot compile on Win32 (native, libpq
> and psql) again. Changes are:
> 1) psql requires the includes of "io.h" and "fcntl.h" in command.c in order
> to make a call to open() work (io.h for _open(), fcntl.h for the O_xxx)
> 2) PG_VERSION is no longer defined in version.h[.in], but in configure.in.
> Since we don't do configure on native win32, we need to put it in
> config.h.win32 :-(
> 3) Added define of SYSCONFDIR to config.h.win32 - libpq won't compile
> without it. This functionality is *NOT* tested - it's just defined as "" for
> now. May work, may not.
> 4) DEF_PGPORT renamed to DEF_PGPORT_STR
> 
> I have done the "basic tests" on it - it connects to a database, and I can
> run queries. Haven't tested any of the fancier functions (yet).
> 
> However, I stepped on a much bigger problem when fixing psql to work. It no
> longer works when linked against the .DLL version of libpq (which the
> Makefile does for it). I have left it linked against this version anyway,
> pending the comments I get on this mail :-)
> The problem is that there are strings being allocated from libpq.dll using
> PQExpBuffers (for example, initPQExpBuffer() on line 92 of input.c). These
> are being allocated using the malloc function used by libpq.dll. This
> function *may* be different from the malloc function used by psql.exe - only
> the resulting pointer must be valid. And with the default linking methods,
> it *WILL* be different. Later, psql.exe tries to free() this string, at
> which point it crashes because the free() function can't find the allocated
> block (it's on the allocated blocks list used by the runtime lib of
> libpq.dll).
> 
> Shouldn't the right thing to do be to have psql call termPQExpBuffer() on
> the data instead? As it is now, gets_fromFile() will just return the pointer
> received from the PQExpBuffer.data (this may well be present at several
> places - this is the one I was bitten by so far). Isn't that kind of
> "accessing the internals of the PQExpBuffer structure" wrong? Instead,
> perhaps it shuold make a copy of the string, adn then termPQExpBuffer() it?
> In that case, the string will have been allocated from within the same
> library as the free() is called.
> 
> I can get it to work just fine by doing this - changing from (around line
> 100 of input.c):
>                 if (buffer.data[buffer.len - 1] == '\n')
>                 {
>                         buffer.data[buffer.len - 1] = '\0';
>                         return buffer.data;
>                 }
> to
>         if (buffer.data[buffer.len - 1] == '\n')
>         {
>             char *tmps;
>             buffer.data[buffer.len - 1] = '\0';
>             tmps = strdup(buffer.data);
>             termPQExpBuffer(&buffer);
>             return tmps;
>         }
> 
> and the same a bit further down in the same function.
> 
> But, as I said above, this may be at more places in the code? Perhaps
> someone more familiar to it could comment on that?
> 
> 
> What do you think shuld be done about this? Personally, I go by the "If you
> allocate a piece of memory using an interface, use the same interface to
> free it", but the question is how to make it work :-)
> 
> 
> Also, AFAIK this only affects psql.exe, so the changes made to the libpq
> files by this patch are required no matter how the other issue is handled.
> 
> Regards,
>  Magnus
> 
> 
>  <<pgsql-win32.patch>> 

[ Attachment, skipping... ]


--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
853-3000+  If your life is a hard drive,     |  830 Blythe Avenue +  Christ can be your backup.        |  Drexel Hill,
Pennsylvania19026