Thread: libpq docs about PQfreemem
have this about PQfreemem(): Frees memory allocated by <application>libpq</>, particularly <function>PQescapeByteaConn</function>, <function>PQescapeBytea</function>, <function>PQunescapeBytea</function>, and <function>PQnotifies</function>. It is neededby Microsoft Windows, which cannot free memory across DLLs, unless multithreaded DLLs (<option>/MD</option> in VC6)are used. On other platforms, this function is the same as the standard library function <function>free()</>. </para> That's also a very old comment, dating back to when we could build libpq with VC6 only and nothing else. Now we can build with MinGW, Borland, VC2005 and I think also VC2003. Which would add the note that this is also depending on *which compiler* and *which compiler version*. The /MD mention is just one of several different options to select the runtime libraries, so it seems really misplaced. Now, there are two options for this. Either we fix it (I can put together a patch), or we remove it altogether. To me, it seems to be just an implementation detail and some kind of explanation why we're doing it - which would live better in a source code comment than in the docs. This includes the part about how it's just the same as free() on other platforms. That's just an implementation detail, and I assume we don't want people to rely on that - in case we ever want to change it in the future for some reason. (the doc for the other functions say you have to use PQfreemem without mentioning any exceptions) Thoughts? Rip out or update? //Magnus
Magnus Hagander wrote: > Now, there are two options for this. Either we fix it (I can put > together a patch), or we remove it altogether. To me, it seems to be > just an implementation detail and some kind of explanation why we're > doing it - which would live better in a source code comment than in > the docs. Old code assumes you can use free() to free all of these things, so it seems reasonable to give some background about why that is not the best method anymore. -- Peter Eisentraut http://developer.postgresql.org/~petere/
Peter Eisentraut wrote: > Magnus Hagander wrote: >> Now, there are two options for this. Either we fix it (I can put >> together a patch), or we remove it altogether. To me, it seems to be >> just an implementation detail and some kind of explanation why we're >> doing it - which would live better in a source code comment than in >> the docs. > > Old code assumes you can use free() to free all of these things, so it > seems reasonable to give some background about why that is not the best > method anymore. Ok. So you say update it to be worded to cover the correct info about win32? Or should we change it into something that says "previously you could use free(), but for portability reasons..."etc? //Magnus
Magnus Hagander wrote: > have this about PQfreemem(): > > Frees memory allocated by <application>libpq</>, particularly > <function>PQescapeByteaConn</function>, > <function>PQescapeBytea</function>, > <function>PQunescapeBytea</function>, > and <function>PQnotifies</function>. > It is needed by Microsoft Windows, which cannot free memory across > DLLs, unless multithreaded DLLs (<option>/MD</option> in VC6) are used. > On other platforms, this function is the same as the standard library > function <function>free()</>. > </para> > > > > That's also a very old comment, dating back to when we could build libpq > with VC6 only and nothing else. Now we can build with MinGW, Borland, > VC2005 and I think also VC2003. Which would add the note that this is > also depending on *which compiler* and *which compiler version*. > The /MD mention is just one of several different options to select the > runtime libraries, so it seems really misplaced. > > Now, there are two options for this. Either we fix it (I can put > together a patch), or we remove it altogether. To me, it seems to be > just an implementation detail and some kind of explanation why we're > doing it - which would live better in a source code comment than in the > docs. > This includes the part about how it's just the same as free() on other > platforms. That's just an implementation detail, and I assume we don't > want people to rely on that - in case we ever want to change it in the > future for some reason. (the doc for the other functions say you have to > use PQfreemem without mentioning any exceptions) > > Thoughts? Rip out or update? Are you saying that almost all Win32 binaries and libraries now can free across DLLs? -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Mon, Feb 05, 2007 at 05:21:34PM -0500, Bruce Momjian wrote: > Magnus Hagander wrote: > > have this about PQfreemem(): > > > > Frees memory allocated by <application>libpq</>, particularly > > <function>PQescapeByteaConn</function>, > > <function>PQescapeBytea</function>, > > <function>PQunescapeBytea</function>, > > and <function>PQnotifies</function>. > > It is needed by Microsoft Windows, which cannot free memory across > > DLLs, unless multithreaded DLLs (<option>/MD</option> in VC6) are used. > > On other platforms, this function is the same as the standard library > > function <function>free()</>. > > </para> > > > > > > > > That's also a very old comment, dating back to when we could build libpq > > with VC6 only and nothing else. Now we can build with MinGW, Borland, > > VC2005 and I think also VC2003. Which would add the note that this is > > also depending on *which compiler* and *which compiler version*. > > The /MD mention is just one of several different options to select the > > runtime libraries, so it seems really misplaced. > > > > Now, there are two options for this. Either we fix it (I can put > > together a patch), or we remove it altogether. To me, it seems to be > > just an implementation detail and some kind of explanation why we're > > doing it - which would live better in a source code comment than in the > > docs. > > This includes the part about how it's just the same as free() on other > > platforms. That's just an implementation detail, and I assume we don't > > want people to rely on that - in case we ever want to change it in the > > future for some reason. (the doc for the other functions say you have to > > use PQfreemem without mentioning any exceptions) > > > > Thoughts? Rip out or update? > > Are you saying that almost all Win32 binaries and libraries now can free > across DLLs? No, I'm saying that the problem is not just between multithreaded and not, it's depending on a lot other factors as well. It's actuallyi the same issue as with PQtrace, which has a better explanation. FOr example, you can't free() a poniter allocated with the MSVC runtime library if you're using the Borland runtime librarys version of free(). //Magnus
> > future for some reason. (the doc for the other functions say you have to > > use PQfreemem without mentioning any exceptions) > > > > Thoughts? Rip out or update? > > Are you saying that almost all Win32 binaries and libraries now can free > across DLLs? You can under very narrow conditions. You need to force code generation for "Multithreaded DLL" run-time libraries (e.g. in VC6 msvcrt.dll) for all exe's and dll's. This is bad for debugging, memory checkers and probably impossible when using different compilers. So you really need PQfreemem. Andreas
I have updated the PQfree documentation; patch attached. Backpatched to 8.2.X. --------------------------------------------------------------------------- Zeugswetter Andreas ADI SD wrote: > > > > future for some reason. (the doc for the other functions say you > have to > > > use PQfreemem without mentioning any exceptions) > > > > > > Thoughts? Rip out or update? > > > > Are you saying that almost all Win32 binaries and libraries now can > free > > across DLLs? > > You can under very narrow conditions. You need to force code generation > for "Multithreaded DLL" run-time libraries (e.g. in VC6 msvcrt.dll) > for all exe's and dll's. > This is bad for debugging, memory checkers and probably impossible > when using different compilers. > So you really need PQfreemem. > > Andreas -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/libpq.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.231 diff -c -c -r1.231 libpq.sgml *** doc/src/sgml/libpq.sgml 16 Feb 2007 16:37:29 -0000 1.231 --- doc/src/sgml/libpq.sgml 19 Feb 2007 22:04:28 -0000 *************** *** 2617,2625 **** <function>PQescapeBytea</function>, <function>PQunescapeBytea</function>, and <function>PQnotifies</function>. ! It is needed by Microsoft Windows, which cannot free memory across ! DLLs, unless multithreaded DLLs (<option>/MD</option> in VC6) are used. ! On other platforms, this function is the same as the standard library function <function>free()</>. </para> </listitem> </varlistentry> --- 2617,2629 ---- <function>PQescapeBytea</function>, <function>PQunescapeBytea</function>, and <function>PQnotifies</function>. ! It is particularly important that this function, rather than ! <function>free()</>, be used on Microsoft Windows. This is because ! allocating memory in a DLL and releasing it in the application works ! only if multithreaded/single-threaded, release/debug, and static/dynamic ! flags are the same for the DLL and the application. On non-Microsoft ! Windows platforms, this function is the same as the standard library ! function <function>free()</>. </para> </listitem> </varlistentry>