Thread: Postgresql on win32

Postgresql on win32

From
Magnus Hagander
Date:
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

Re: Postgresql on win32

From
Bruce Momjian
Date:
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