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(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026