Thread: PQescapeBytea on Win32
Hi - there is a problem with PQescapeBytea for Win32. Since libpq is a DLL, all memory allocated from within the DLL needs to be freed from within the dll. PQescapeBytea allocates memory, but there is no function call back into the DLL to free this memory. This causes heap corruption when the main program tries to free the memory itself. The alternative is to just leak the memory and never free it. Also bad.... -Dave Brown key88sf@hotmail.com _________________________________________________________________ Add photos to your messages with MSN 8. Get 2 months FREE*. http://join.msn.com/?page=features/featuredemail
Yes, I am aware of that limitation. If you link libpq as a Multithreaded DLL, it will not link libc into each DLL, but have only one libc that can free from anywhere. Is that acceptable or do we need a Win32 specific memory free function? --------------------------------------------------------------------------- Key88 SF wrote: > > Hi - there is a problem with PQescapeBytea for Win32. Since libpq is a DLL, > all memory allocated from within the DLL needs to be freed from within the > dll. > > PQescapeBytea allocates memory, but there is no function call back into the > DLL to free this memory. This causes heap corruption when the main program > tries to free the memory itself. The alternative is to just leak the memory > and never free it. Also bad.... > > -Dave Brown > key88sf@hotmail.com > > > _________________________________________________________________ > Add photos to your messages with MSN 8. Get 2 months FREE*. > http://join.msn.com/?page=features/featuredemail > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
I suppose that is ok. But the default compiler flags in the provided makefile for libpq is non-multithreaded. That should at least be fixed I would think. If it's not a big deal for you, I would say that in the name of safety for those people who don't look closely at these things, it would be better to have the callback function to delete the memory and not have to assume libpq is built with the correct c-runtimes... -Dave >From: Bruce Momjian <pgman@candle.pha.pa.us> >To: Key88 SF <key88sf@hotmail.com> >CC: pgsql-hackers@postgresql.org >Subject: Re: [HACKERS] PQescapeBytea on Win32 >Date: Tue, 18 Mar 2003 00:01:38 -0500 (EST) >MIME-Version: 1.0 >Received: from candle.pha.pa.us ([207.106.42.251]) by >mc6-f24.law1.hotmail.com with Microsoft SMTPSVC(5.0.2195.5600); Mon, 17 Mar >2003 21:01:44 -0800 >Received: (from pgman@localhost)by candle.pha.pa.us (8.11.6/8.10.1) id >h2I51cW03568;Tue, 18 Mar 2003 00:01:38 -0500 (EST) >X-Message-Info: JGTYoYF78jEHjJx36Oi8+Q1OJDRSDidP >Message-Id: <200303180501.h2I51cW03568@candle.pha.pa.us> >In-Reply-To: <F159qHSgNZuILS94O9J00042a23@hotmail.com> >X-Mailer: ELM [version 2.4ME+ PL99 (25)] >Return-Path: pgman@candle.pha.pa.us >X-OriginalArrivalTime: 18 Mar 2003 05:01:44.0926 (UTC) >FILETIME=[78F0B7E0:01C2ED0B] > > >Yes, I am aware of that limitation. If you link libpq as a >Multithreaded DLL, it will not link libc into each DLL, but have only >one libc that can free from anywhere. > >Is that acceptable or do we need a Win32 specific memory free function? > >--------------------------------------------------------------------------- > >Key88 SF wrote: > > > > Hi - there is a problem with PQescapeBytea for Win32. Since libpq is a >DLL, > > all memory allocated from within the DLL needs to be freed from within >the > > dll. > > > > PQescapeBytea allocates memory, but there is no function call back into >the > > DLL to free this memory. This causes heap corruption when the main >program > > tries to free the memory itself. The alternative is to just leak the >memory > > and never free it. Also bad.... > > > > -Dave Brown > > key88sf@hotmail.com > > > > > > _________________________________________________________________ > > Add photos to your messages with MSN 8. Get 2 months FREE*. > > http://join.msn.com/?page=features/featuredemail > > > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 3: if posting/reading through Usenet, please send an appropriate > > subscribe-nomail command to majordomo@postgresql.org so that your > > message can get through to the mailing list cleanly > > > >-- > Bruce Momjian | http://candle.pha.pa.us > pgman@candle.pha.pa.us | (610) 359-1001 > + If your life is a hard drive, | 13 Roberts Road > + Christ can be your backup. | Newtown Square, Pennsylvania >19073 _________________________________________________________________ Add photos to your e-mail with MSN 8. Get 2 months FREE*. http://join.msn.com/?page=features/featuredemail
Actually this isn't even working for me. I just compiled the libpq dll with /MT (multithreaded), and I still get corruption in my app. I even tried libpq compiled with multithreaded-debug (/MTd), which my app is using now, and still same problem. I think a separate function would be great to have. >From: "Key88 SF" <key88sf@hotmail.com> >To: pgman@candle.pha.pa.us >CC: pgsql-hackers@postgresql.org >Subject: Re: [HACKERS] PQescapeBytea on Win32 >Date: Tue, 18 Mar 2003 05:07:29 +0000 >MIME-Version: 1.0 >X-Originating-IP: [67.118.28.93] >Received: from relay1.pgsql.com ([64.49.215.129]) by >mc5-f2.law1.hotmail.com with Microsoft SMTPSVC(5.0.2195.5600); Mon, 17 Mar >2003 21:07:40 -0800 >Received: from postgresql.org (postgresql.org [64.49.215.8])by >relay1.pgsql.com (Postfix) with ESMTPid DB3426F73A1; Tue, 18 Mar 2003 >00:07:38 -0500 (EST) >Received: from hotmail.com (f127.law9.hotmail.com [64.4.9.127])by >postgresql.org (Postfix) with ESMTP id 0247A474E4Ffor ><pgsql-hackers@postgresql.org>; Tue, 18 Mar 2003 00:07:25 -0500 (EST) >Received: from mail pickup service by hotmail.com with Microsoft SMTPSVC; >Mon, 17 Mar 2003 21:07:30 -0800 >Received: from 67.118.28.93 by lw9fd.law9.hotmail.msn.com with HTTP;Tue, 18 >Mar 2003 05:07:29 GMT >X-Message-Info: JGTYoYF78jEHjJx36Oi8+Q1OJDRSDidP >X-Original-To: pgsql-hackers@postgresql.org >Message-ID: <F127CGlbZvFqV2mEIkL00001ff2@hotmail.com> >X-OriginalArrivalTime: 18 Mar 2003 05:07:30.0217 (UTC) >FILETIME=[46BFF190:01C2ED0C] >Precedence: bulk >Sender: pgsql-hackers-owner@postgresql.org >Return-Path: pgsql-hackers-owner+M36954@postgresql.org > >I suppose that is ok. But the default compiler flags in the provided >makefile for libpq is non-multithreaded. That should at least be fixed I >would think. > >If it's not a big deal for you, I would say that in the name of safety for >those people who don't look closely at these things, it would be better to >have the callback function to delete the memory and not have to assume >libpq is built with the correct c-runtimes... > >-Dave > > > > >>From: Bruce Momjian <pgman@candle.pha.pa.us> >>To: Key88 SF <key88sf@hotmail.com> >>CC: pgsql-hackers@postgresql.org >>Subject: Re: [HACKERS] PQescapeBytea on Win32 >>Date: Tue, 18 Mar 2003 00:01:38 -0500 (EST) >>MIME-Version: 1.0 >>Received: from candle.pha.pa.us ([207.106.42.251]) by >>mc6-f24.law1.hotmail.com with Microsoft SMTPSVC(5.0.2195.5600); Mon, 17 >>Mar 2003 21:01:44 -0800 >>Received: (from pgman@localhost)by candle.pha.pa.us (8.11.6/8.10.1) id >>h2I51cW03568;Tue, 18 Mar 2003 00:01:38 -0500 (EST) >>X-Message-Info: JGTYoYF78jEHjJx36Oi8+Q1OJDRSDidP >>Message-Id: <200303180501.h2I51cW03568@candle.pha.pa.us> >>In-Reply-To: <F159qHSgNZuILS94O9J00042a23@hotmail.com> >>X-Mailer: ELM [version 2.4ME+ PL99 (25)] >>Return-Path: pgman@candle.pha.pa.us >>X-OriginalArrivalTime: 18 Mar 2003 05:01:44.0926 (UTC) >>FILETIME=[78F0B7E0:01C2ED0B] >> >> >>Yes, I am aware of that limitation. If you link libpq as a >>Multithreaded DLL, it will not link libc into each DLL, but have only >>one libc that can free from anywhere. >> >>Is that acceptable or do we need a Win32 specific memory free function? >> >>--------------------------------------------------------------------------- >> >>Key88 SF wrote: >> > >> > Hi - there is a problem with PQescapeBytea for Win32. Since libpq is a >>DLL, >> > all memory allocated from within the DLL needs to be freed from within >>the >> > dll. >> > >> > PQescapeBytea allocates memory, but there is no function call back into >>the >> > DLL to free this memory. This causes heap corruption when the main >>program >> > tries to free the memory itself. The alternative is to just leak the >>memory >> > and never free it. Also bad.... >> > >> > -Dave Brown >> > key88sf@hotmail.com >> > >> > >> > _________________________________________________________________ >> > Add photos to your messages with MSN 8. Get 2 months FREE*. >> > http://join.msn.com/?page=features/featuredemail >> > >> > >> > ---------------------------(end of >>broadcast)--------------------------- >> > TIP 3: if posting/reading through Usenet, please send an appropriate >> > subscribe-nomail command to majordomo@postgresql.org so that your >> > message can get through to the mailing list cleanly >> > >> >>-- >> Bruce Momjian | http://candle.pha.pa.us >> pgman@candle.pha.pa.us | (610) 359-1001 >> + If your life is a hard drive, | 13 Roberts Road >> + Christ can be your backup. | Newtown Square, Pennsylvania >>19073 > > >_________________________________________________________________ >Add photos to your e-mail with MSN 8. Get 2 months FREE*. >http://join.msn.com/?page=features/featuredemail > > >---------------------------(end of broadcast)--------------------------- >TIP 2: you can get off all lists at once with the unregister command > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) _________________________________________________________________ Protect your PC - get McAfee.com VirusScan Online http://clinic.mcafee.com/clinic/ibuy/campaign.asp?cid=3963
Key88 SF wrote: > I suppose that is ok. But the default compiler flags in the provided > makefile for libpq is non-multithreaded. That should at least be fixed I > would think. > > If it's not a big deal for you, I would say that in the name of safety > for those people who don't look closely at these things, it would be > better to have the callback function to delete the memory and not have > to assume libpq is built with the correct c-runtimes... > Since you know exactly what's needed, and a project that needs it, why don't you submit a patch? ;-) Joe
All apps need the multi-threaded option, I think. --------------------------------------------------------------------------- Key88 SF wrote: > > Actually this isn't even working for me. I just compiled the libpq dll with > /MT (multithreaded), and I still get corruption in my app. I even tried > libpq compiled with multithreaded-debug (/MTd), which my app is using now, > and still same problem. I think a separate function would be great to have. > > > >From: "Key88 SF" <key88sf@hotmail.com> > >To: pgman@candle.pha.pa.us > >CC: pgsql-hackers@postgresql.org > >Subject: Re: [HACKERS] PQescapeBytea on Win32 > >Date: Tue, 18 Mar 2003 05:07:29 +0000 > >MIME-Version: 1.0 > >X-Originating-IP: [67.118.28.93] > >Received: from relay1.pgsql.com ([64.49.215.129]) by > >mc5-f2.law1.hotmail.com with Microsoft SMTPSVC(5.0.2195.5600); Mon, 17 Mar > >2003 21:07:40 -0800 > >Received: from postgresql.org (postgresql.org [64.49.215.8])by > >relay1.pgsql.com (Postfix) with ESMTPid DB3426F73A1; Tue, 18 Mar 2003 > >00:07:38 -0500 (EST) > >Received: from hotmail.com (f127.law9.hotmail.com [64.4.9.127])by > >postgresql.org (Postfix) with ESMTP id 0247A474E4Ffor > ><pgsql-hackers@postgresql.org>; Tue, 18 Mar 2003 00:07:25 -0500 (EST) > >Received: from mail pickup service by hotmail.com with Microsoft SMTPSVC; > >Mon, 17 Mar 2003 21:07:30 -0800 > >Received: from 67.118.28.93 by lw9fd.law9.hotmail.msn.com with HTTP;Tue, 18 > >Mar 2003 05:07:29 GMT > >X-Message-Info: JGTYoYF78jEHjJx36Oi8+Q1OJDRSDidP > >X-Original-To: pgsql-hackers@postgresql.org > >Message-ID: <F127CGlbZvFqV2mEIkL00001ff2@hotmail.com> > >X-OriginalArrivalTime: 18 Mar 2003 05:07:30.0217 (UTC) > >FILETIME=[46BFF190:01C2ED0C] > >Precedence: bulk > >Sender: pgsql-hackers-owner@postgresql.org > >Return-Path: pgsql-hackers-owner+M36954@postgresql.org > > > >I suppose that is ok. But the default compiler flags in the provided > >makefile for libpq is non-multithreaded. That should at least be fixed I > >would think. > > > >If it's not a big deal for you, I would say that in the name of safety for > >those people who don't look closely at these things, it would be better to > >have the callback function to delete the memory and not have to assume > >libpq is built with the correct c-runtimes... > > > >-Dave > > > > > > > > > >>From: Bruce Momjian <pgman@candle.pha.pa.us> > >>To: Key88 SF <key88sf@hotmail.com> > >>CC: pgsql-hackers@postgresql.org > >>Subject: Re: [HACKERS] PQescapeBytea on Win32 > >>Date: Tue, 18 Mar 2003 00:01:38 -0500 (EST) > >>MIME-Version: 1.0 > >>Received: from candle.pha.pa.us ([207.106.42.251]) by > >>mc6-f24.law1.hotmail.com with Microsoft SMTPSVC(5.0.2195.5600); Mon, 17 > >>Mar 2003 21:01:44 -0800 > >>Received: (from pgman@localhost)by candle.pha.pa.us (8.11.6/8.10.1) id > >>h2I51cW03568;Tue, 18 Mar 2003 00:01:38 -0500 (EST) > >>X-Message-Info: JGTYoYF78jEHjJx36Oi8+Q1OJDRSDidP > >>Message-Id: <200303180501.h2I51cW03568@candle.pha.pa.us> > >>In-Reply-To: <F159qHSgNZuILS94O9J00042a23@hotmail.com> > >>X-Mailer: ELM [version 2.4ME+ PL99 (25)] > >>Return-Path: pgman@candle.pha.pa.us > >>X-OriginalArrivalTime: 18 Mar 2003 05:01:44.0926 (UTC) > >>FILETIME=[78F0B7E0:01C2ED0B] > >> > >> > >>Yes, I am aware of that limitation. If you link libpq as a > >>Multithreaded DLL, it will not link libc into each DLL, but have only > >>one libc that can free from anywhere. > >> > >>Is that acceptable or do we need a Win32 specific memory free function? > >> > >>--------------------------------------------------------------------------- > >> > >>Key88 SF wrote: > >> > > >> > Hi - there is a problem with PQescapeBytea for Win32. Since libpq is a > >>DLL, > >> > all memory allocated from within the DLL needs to be freed from within > >>the > >> > dll. > >> > > >> > PQescapeBytea allocates memory, but there is no function call back into > >>the > >> > DLL to free this memory. This causes heap corruption when the main > >>program > >> > tries to free the memory itself. The alternative is to just leak the > >>memory > >> > and never free it. Also bad.... > >> > > >> > -Dave Brown > >> > key88sf@hotmail.com > >> > > >> > > >> > _________________________________________________________________ > >> > Add photos to your messages with MSN 8. Get 2 months FREE*. > >> > http://join.msn.com/?page=features/featuredemail > >> > > >> > > >> > ---------------------------(end of > >>broadcast)--------------------------- > >> > TIP 3: if posting/reading through Usenet, please send an appropriate > >> > subscribe-nomail command to majordomo@postgresql.org so that your > >> > message can get through to the mailing list cleanly > >> > > >> > >>-- > >> Bruce Momjian | http://candle.pha.pa.us > >> pgman@candle.pha.pa.us | (610) 359-1001 > >> + If your life is a hard drive, | 13 Roberts Road > >> + Christ can be your backup. | Newtown Square, Pennsylvania > >>19073 > > > > > >_________________________________________________________________ > >Add photos to your e-mail with MSN 8. Get 2 months FREE*. > >http://join.msn.com/?page=features/featuredemail > > > > > >---------------------------(end of broadcast)--------------------------- > >TIP 2: you can get off all lists at once with the unregister command > > (send "unregister YourEmailAddressHere" to majordomo@postgresql.org) > > > _________________________________________________________________ > Protect your PC - get McAfee.com VirusScan Online > http://clinic.mcafee.com/clinic/ibuy/campaign.asp?cid=3963 > > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Sure - I figured since it's a 1-line function, someone would just write it themselves in the next release. But here it is attached. I'm not sure exactly how you like your patches submitted, so here is a zip file with the modified libpq-fe.h and fe-exec.c files. The only change is the addition of: PQescapeByteaFree(unsigned char *); -Dave >From: Joe Conway <mail@joeconway.com> >To: Key88 SF <key88sf@hotmail.com> >CC: pgman@candle.pha.pa.us, pgsql-hackers@postgresql.org >Subject: Re: [HACKERS] PQescapeBytea on Win32 >Date: Mon, 17 Mar 2003 21:41:26 -0800 >MIME-Version: 1.0 >Received: from relay1.pgsql.com ([64.49.215.129]) by >mc8-f12.law1.hotmail.com with Microsoft SMTPSVC(5.0.2195.5600); Mon, 17 Mar >2003 21:43:59 -0800 >Received: from postgresql.org (postgresql.org [64.49.215.8])by >relay1.pgsql.com (Postfix) with ESMTPid 33CF26F7766; Tue, 18 Mar 2003 >00:43:58 -0500 (EST) >Received: from joeconway.com (66-146-172-86.skyriver.net [66.146.172.86])by >postgresql.org (Postfix) with ESMTP id 4CC53474E4Ffor ><pgsql-hackers@postgresql.org>; Tue, 18 Mar 2003 00:43:47 -0500 (EST) >Received: from [192.168.5.3] (account jconway HELO joeconway.com) by >joeconway.com (CommuniGate Pro SMTP 4.0.4) with ESMTP-TLS id 1667227; Mon, >17 Mar 2003 22:19:54 -0800 >X-Message-Info: JGTYoYF78jEHjJx36Oi8+Q1OJDRSDidP >X-Original-To: pgsql-hackers@postgresql.org >Message-ID: <3E76B186.2050407@joeconway.com> >User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.2.1) >Gecko/20021130 >X-Accept-Language: en-us, en >References: <F127CGlbZvFqV2mEIkL00001ff2@hotmail.com> >In-Reply-To: <F127CGlbZvFqV2mEIkL00001ff2@hotmail.com> >Precedence: bulk >Sender: pgsql-hackers-owner@postgresql.org >Return-Path: pgsql-hackers-owner+M36957@postgresql.org >X-OriginalArrivalTime: 18 Mar 2003 05:44:00.0021 (UTC) >FILETIME=[5FF98450:01C2ED11] > >Key88 SF wrote: >>I suppose that is ok. But the default compiler flags in the provided >>makefile for libpq is non-multithreaded. That should at least be fixed I >>would think. >> >>If it's not a big deal for you, I would say that in the name of safety for >>those people who don't look closely at these things, it would be better to >>have the callback function to delete the memory and not have to assume >>libpq is built with the correct c-runtimes... >> > >Since you know exactly what's needed, and a project that needs it, why >don't you submit a patch? ;-) > >Joe > > > > >---------------------------(end of broadcast)--------------------------- >TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org _________________________________________________________________ The new MSN 8: smart spam protection and 2 months FREE* http://join.msn.com/?page=features/junkmail
Attachment
> Actually this isn't even working for me. I just compiled the libpq dll with > /MT (multithreaded), and I still get corruption in my app. I even tried > libpq compiled with multithreaded-debug (/MTd), which my app is using now, > and still same problem. No, you need to use multithreaded DLL (/MD in VC6) for both the dll and your program, then it works. But since that is not optimal (can't use debug runtime unless you have a second debug dll (/MDd)), I also think a callback function would be good to have in addition to linking libpq.dll with /MD. Informix e.g. supplies SqlFreeMem( void * MemAddr, int FreeType ); Andreas
I have modified the patch to call it PQfreemem(), in case there are other cases we need to free libpq memory. Patch attached and applied. --------------------------------------------------------------------------- Zeugswetter Andreas SB SD wrote: > > > Actually this isn't even working for me. I just compiled the libpq dll with > > /MT (multithreaded), and I still get corruption in my app. I even tried > > libpq compiled with multithreaded-debug (/MTd), which my app is using now, > > and still same problem. > > No, you need to use multithreaded DLL (/MD in VC6) for both the dll and your program, > then it works. > But since that is not optimal (can't use debug runtime unless you have a second > debug dll (/MDd)), I also think a callback function would be good to have in addition > to linking libpq.dll with /MD. > > Informix e.g. supplies SqlFreeMem( void * MemAddr, int FreeType ); > > Andreas > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/libpq.sgml =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/libpq.sgml,v retrieving revision 1.113 diff -c -c -r1.113 libpq.sgml *** doc/src/sgml/libpq.sgml 20 Mar 2003 06:23:30 -0000 1.113 --- doc/src/sgml/libpq.sgml 22 Mar 2003 03:27:47 -0000 *************** *** 1151,1156 **** --- 1151,1175 ---- </para> </listitem> </varlistentry> + + <varlistentry> + <term><function>PQfreemem</function></term> + <listitem> + <para> + Frees memory allocated by <application>libpq</> + <synopsis> + void PQfreemem(void *ptr); + </synopsis> + </para> + + <para> + Frees memory allocated by <application>libpq</>, particularly + <function>PQescapeBytea</function> and <function>PQunescapeBytea</function>. + It is needed by Win32, which can not free memory across + DLL's, unless multithreaded DLL's (/MD in VC6) are used. + </para> + </listitem> + </varlistentry> </variablelist> </sect2> Index: src/interfaces/libpq/fe-exec.c =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.126 diff -c -c -r1.126 fe-exec.c *** src/interfaces/libpq/fe-exec.c 10 Mar 2003 22:28:21 -0000 1.126 --- src/interfaces/libpq/fe-exec.c 22 Mar 2003 03:27:53 -0000 *************** *** 284,289 **** --- 284,302 ---- return buffer; } + + /* + * PQfreemem - safely frees memory allocated + * + * Needed mostly by Win32, unless multithreaded DLL (/MD in VC6) + * Used for freeing memory from PQescapeByte()a/PQunescapeBytea() + */ + void PQfreemem(void *ptr) + { + free(ptr); + } + + /* ---------------- * Space management for PGresult. * Index: src/interfaces/libpq/libpq-fe.h =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.89 diff -c -c -r1.89 libpq-fe.h *** src/interfaces/libpq/libpq-fe.h 20 Mar 2003 06:23:30 -0000 1.89 --- src/interfaces/libpq/libpq-fe.h 22 Mar 2003 03:27:54 -0000 *************** *** 254,259 **** --- 254,260 ---- size_t *bytealen); extern unsigned char *PQunescapeBytea(const unsigned char *strtext, size_t *retbuflen); + extern void PQfreemem(void *ptr); /* Simple synchronous query */
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I have modified the patch to call it PQfreemem(), in case there are other
> cases we need to free libpq memory.
Doesn't this duplicate a function that we already invented for PQnotify
structs?
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I have modified the patch to call it PQfreemem(), in case there are other
> > cases we need to free libpq memory.
>
> Doesn't this duplicate a function that we already invented for PQnotify
> structs?
Yep, I see it now:/* * PQfreeNotify - free's the memory associated with a PGnotify * * This function is needed on
Windowswhen using libpq.dll and * for example libpgtcl.dll: All memory allocated inside a dll * should be freed in the
contextof the same dll. * */voidPQfreeNotify(PGnotify *notify){ free(notify);}
What do you recommend? Do we depricate PQfreeNotify?
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Doesn't this duplicate a function that we already invented for PQnotify
>> structs?
> What do you recommend? Do we depricate PQfreeNotify?
I dunno. In hindsight it was shortsightedly named. But I don't suppose
we want to tell people to use "PQfreeNotify" to free the result of
PQescapeBytea ... so deprecation seems to be the order of the day.
Do we really want "PQfreemem" either? Maybe it should be "PQfree"?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Doesn't this duplicate a function that we already invented for PQnotify > >> structs? > > > What do you recommend? Do we depricate PQfreeNotify? > > I dunno. In hindsight it was shortsightedly named. But I don't suppose > we want to tell people to use "PQfreeNotify" to free the result of > PQescapeBytea ... so deprecation seems to be the order of the day. > > Do we really want "PQfreemem" either? Maybe it should be "PQfree"? I am a little concerned that PQfree would be confused with PQclear. Could we have PQfreeNotify() be a macro to PQfreemem in 7.4? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote: > Tom Lane wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > Tom Lane wrote: > > >> Doesn't this duplicate a function that we already invented for PQnotify > > >> structs? > > > > > What do you recommend? Do we depricate PQfreeNotify? > > > > I dunno. In hindsight it was shortsightedly named. But I don't suppose > > we want to tell people to use "PQfreeNotify" to free the result of > > PQescapeBytea ... so deprecation seems to be the order of the day. > > > > Do we really want "PQfreemem" either? Maybe it should be "PQfree"? > > I am a little concerned that PQfree would be confused with PQclear. > > Could we have PQfreeNotify() be a macro to PQfreemem in 7.4? The only place I see PQfreeNotify() used in our code is libpgtcl, aside from libpq, and a macro would work fine. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> Do we really want "PQfreemem" either? Maybe it should be "PQfree"?
> I am a little concerned that PQfree would be confused with PQclear.
Good point --- nevermind that suggestion.
> Could we have PQfreeNotify() be a macro to PQfreemem in 7.4?
I'd like to do that but I doubt we can get away with it; it'd be
an incompatible change at the binary API level, forcing a major
version bump on libpq.so. (On the other hand, we might wind up
doing one anyway; not sure.)
You could do it the other way: PQfreemem a macro for PQfreeNotify.
That would stay binary-compatible with existing clients. We could
switch which one is a macro at the next occasion for a major
version bump on the shlib, and then eventually remove PQfreeNotify
when we feel it's been deprecated for long enough.
regards, tom lane
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> Do we really want "PQfreemem" either? Maybe it should be "PQfree"?
>
> > I am a little concerned that PQfree would be confused with PQclear.
>
> Good point --- nevermind that suggestion.
>
> > Could we have PQfreeNotify() be a macro to PQfreemem in 7.4?
>
> I'd like to do that but I doubt we can get away with it; it'd be
> an incompatible change at the binary API level, forcing a major
> version bump on libpq.so. (On the other hand, we might wind up
> doing one anyway; not sure.)
Right. We changed the Notify internal structure and got bitten on that
in 7.3 -- don't want that to happen again.
> You could do it the other way: PQfreemem a macro for PQfreeNotify.
> That would stay binary-compatible with existing clients. We could
> switch which one is a macro at the next occasion for a major
> version bump on the shlib, and then eventually remove PQfreeNotify
> when we feel it's been deprecated for long enough.
The problem with that is the new versions are still going to reference
PQfreeNotify, and then we still can't remove it. I think we need the
macro for PQfreeNotify pointing to PQfreemem, but keep the PQfreeNotify
function around for a release or two, then remove it, and remove all
documentation about it in 7.4.
I now see that psql doesn't use that call, though it should if it is
running on Win32:static voidPrintNotifications(void){ PGnotify *notify; while ((notify = PQnotifies(pset.db)))
{ fprintf(pset.queryFout, gettext("Asynchronous NOTIFY '%s' from backend with pid %d received.\n"),
notify->relname, notify->be_pid); free(notify); fflush(pset.queryFout); }}
I will clean that up too, and check any other interfaces/apps.
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> The problem with that is the new versions are still going to reference
> PQfreeNotify, and then we still can't remove it. I think we need the
> macro for PQfreeNotify pointing to PQfreemem, but keep the PQfreeNotify
> function around for a release or two, then remove it, and remove all
> documentation about it in 7.4.
Hm. You could possibly do that with some judicious #undef'ing ...
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > The problem with that is the new versions are still going to reference > > PQfreeNotify, and then we still can't remove it. I think we need the > > macro for PQfreeNotify pointing to PQfreemem, but keep the PQfreeNotify > > function around for a release or two, then remove it, and remove all > > documentation about it in 7.4. > > Hm. You could possibly do that with some judicious #undef'ing ... Exactly! -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian writes: > The problem with that is the new versions are still going to reference > PQfreeNotify, and then we still can't remove it. I think we need the > macro for PQfreeNotify pointing to PQfreemem, but keep the PQfreeNotify > function around for a release or two, then remove it, and remove all > documentation about it in 7.4. Too complicated. Just implement PQfreemem independently and tell people to use it. Please don't remove documentation just because a feature is deprecated. How are people supposed to do the conversion if they can't find information about what they are supposed to convert from? -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote: > Bruce Momjian writes: > > > The problem with that is the new versions are still going to reference > > PQfreeNotify, and then we still can't remove it. I think we need the > > macro for PQfreeNotify pointing to PQfreemem, but keep the PQfreeNotify > > function around for a release or two, then remove it, and remove all > > documentation about it in 7.4. > > Too complicated. Just implement PQfreemem independently and tell people > to use it. Please don't remove documentation just because a feature is > deprecated. How are people supposed to do the conversion if they can't > find information about what they are supposed to convert from? OK, I will keep the docs around and point to PGfreemem. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
OK, patch applied to document PQfreemem() for notify. PQfreeNotify wasn't even documented, but I kept it in for binary compatibility, and added a #define to map it to PQfreemem(). I updated various interfaces to use PQfreemem() rather than free(). --------------------------------------------------------------------------- Peter Eisentraut wrote: > Bruce Momjian writes: > > > The problem with that is the new versions are still going to reference > > PQfreeNotify, and then we still can't remove it. I think we need the > > macro for PQfreeNotify pointing to PQfreemem, but keep the PQfreeNotify > > function around for a release or two, then remove it, and remove all > > documentation about it in 7.4. > > Too complicated. Just implement PQfreemem independently and tell people > to use it. Please don't remove documentation just because a feature is > deprecated. How are people supposed to do the conversion if they can't > find information about what they are supposed to convert from? > > -- > Peter Eisentraut peter_e@gmx.net > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/libpq.sgml =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/libpq.sgml,v retrieving revision 1.115 diff -c -c -r1.115 libpq.sgml *** doc/src/sgml/libpq.sgml 24 Mar 2003 18:33:52 -0000 1.115 --- doc/src/sgml/libpq.sgml 25 Mar 2003 02:40:07 -0000 *************** *** 1049,1056 **** <function>PQescapeString</> performs this operation. </para> <para> ! The ! parameter <parameter>from</> points to the first character of the string that is to be escaped, and the <parameter>length</> parameter counts the number of characters in this string. (A terminating zero byte is neither necessary nor counted.) <parameter>to</> shall point to a --- 1049,1055 ---- <function>PQescapeString</> performs this operation. </para> <para> ! The parameter <parameter>from</> points to the first character of the string that is to be escaped, and the <parameter>length</> parameter counts the number of characters in this string. (A terminating zero byte is neither necessary nor counted.) <parameter>to</> shall point to a *************** *** 1117,1123 **** <para> <function>PQescapeBytea</> returns an escaped version of the ! <parameter>from</parameter> parameter binary string in memory allocated with <function>malloc()</>. The return string has all special characters replaced so that they can be properly processed by the PostgreSQL string literal parser, and the <type>bytea</type> input function. A terminating zero --- 1116,1124 ---- <para> <function>PQescapeBytea</> returns an escaped version of the ! <parameter>from</parameter> parameter binary string in memory ! allocated with <function>malloc()</>, and must be freed using ! <function>PQfreemem()</>. The return string has all special characters replaced so that they can be properly processed by the PostgreSQL string literal parser, and the <type>bytea</type> input function. A terminating zero *************** *** 1143,1150 **** such as might be returned by <function>PQgetvalue</function> when applied to a <type>bytea</type> column. <function>PQunescapeBytea</function> converts this string representation into its binary representation. ! It returns a pointer to a buffer allocated with <function>malloc()</function>, or null on error, and puts the size ! of the buffer in <parameter>to_length</parameter>. </para> </listitem> </varlistentry> --- 1144,1154 ---- such as might be returned by <function>PQgetvalue</function> when applied to a <type>bytea</type> column. <function>PQunescapeBytea</function> converts this string representation into its binary representation. ! It returns a pointer to a buffer allocated with ! <function>malloc()</function>, or null on error, and puts the size of ! the buffer in <parameter>to_length</parameter>. The memory must be ! freed using <function>PQfreemem()</>. ! </para> </listitem> </varlistentry> *************** *** 1161,1167 **** <para> Frees memory allocated by <application>libpq</>, particularly ! <function>PQescapeBytea</function> and <function>PQunescapeBytea</function>. It is needed by Win32, which can not free memory across DLL's, unless multithreaded DLL's (/MD in VC6) are used. </para> --- 1165,1173 ---- <para> Frees memory allocated by <application>libpq</>, particularly ! <function>PQescapeBytea</function>, ! <function>PQunescapeBytea</function>, ! and <function>PQnotifies</function>. It is needed by Win32, which can not free memory across DLL's, unless multithreaded DLL's (/MD in VC6) are used. </para> *************** *** 1926,1932 **** } PGnotify; </synopsis> After processing a <structname>PGnotify</structname> object returned by <function>PQnotifies</function>, ! be sure to free it with <function>free()</function> to avoid a memory leak. </para> <note> --- 1932,1938 ---- } PGnotify; </synopsis> After processing a <structname>PGnotify</structname> object returned by <function>PQnotifies</function>, ! be sure to free it with <function>PQfreemem()</function>. </para> <note> *************** *** 2867,2873 **** fprintf(stderr, "ASYNC NOTIFY of '%s' from backend pid '%d' received\n", notify->relname, notify->be_pid); ! free(notify); } } --- 2873,2879 ---- fprintf(stderr, "ASYNC NOTIFY of '%s' from backend pid '%d' received\n", notify->relname, notify->be_pid); ! PQfreemem(notify); } } Index: src/bin/psql/common.c =================================================================== RCS file: /cvsroot/pgsql-server/src/bin/psql/common.c,v retrieving revision 1.61 diff -c -c -r1.61 common.c *** src/bin/psql/common.c 20 Mar 2003 15:39:52 -0000 1.61 --- src/bin/psql/common.c 25 Mar 2003 02:40:08 -0000 *************** *** 441,447 **** { fprintf(pset.queryFout, gettext("Asynchronous NOTIFY '%s' from backend with pid %d received.\n"), notify->relname, notify->be_pid); ! free(notify); fflush(pset.queryFout); } } --- 441,447 ---- { fprintf(pset.queryFout, gettext("Asynchronous NOTIFY '%s' from backend with pid %d received.\n"), notify->relname, notify->be_pid); ! PQfreemem(notify); fflush(pset.queryFout); } } Index: src/interfaces/ecpg/ecpglib/execute.c =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/ecpg/ecpglib/execute.c,v retrieving revision 1.4 diff -c -c -r1.4 execute.c *** src/interfaces/ecpg/ecpglib/execute.c 20 Mar 2003 15:56:50 -0000 1.4 --- src/interfaces/ecpg/ecpglib/execute.c 25 Mar 2003 02:40:10 -0000 *************** *** 1197,1203 **** { ECPGlog("ECPGexecute line %d: ASYNC NOTIFY of '%s' from backend pid '%d' received\n", stmt->lineno, notify->relname, notify->be_pid); ! ECPGfree(notify); } return status; --- 1197,1203 ---- { ECPGlog("ECPGexecute line %d: ASYNC NOTIFY of '%s' from backend pid '%d' received\n", stmt->lineno, notify->relname, notify->be_pid); ! PQfreemem(notify); } return status; Index: src/interfaces/libpgtcl/pgtclId.c =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/libpgtcl/pgtclId.c,v retrieving revision 1.40 diff -c -c -r1.40 pgtclId.c *** src/interfaces/libpgtcl/pgtclId.c 1 Feb 2003 00:22:12 -0000 1.40 --- src/interfaces/libpgtcl/pgtclId.c 25 Mar 2003 02:40:11 -0000 *************** *** 554,560 **** if (event->connid == NULL) { if (event->notify) ! PQfreeNotify(event->notify); return 1; } --- 554,560 ---- if (event->connid == NULL) { if (event->notify) ! PQfreemem(event->notify); return 1; } *************** *** 634,640 **** Tcl_Release((ClientData) event->connid); if (event->notify) ! PQfreeNotify(event->notify); return 1; } --- 634,640 ---- Tcl_Release((ClientData) event->connid); if (event->notify) ! PQfreemem(event->notify); return 1; } Index: src/interfaces/libpq/fe-exec.c =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.127 diff -c -c -r1.127 fe-exec.c *** src/interfaces/libpq/fe-exec.c 22 Mar 2003 03:29:06 -0000 1.127 --- src/interfaces/libpq/fe-exec.c 25 Mar 2003 02:40:15 -0000 *************** *** 1578,1597 **** } /* - * PQfreeNotify - free's the memory associated with a PGnotify - * - * This function is needed on Windows when using libpq.dll and - * for example libpgtcl.dll: All memory allocated inside a dll - * should be freed in the context of the same dll. - * - */ - void - PQfreeNotify(PGnotify *notify) - { - free(notify); - } - - /* * PQgetline - gets a newline-terminated string from the backend. * * Chiefly here so that applications can use "COPY <rel> to stdout" --- 1578,1583 ---- *************** *** 2470,2472 **** --- 2456,2477 ---- { return pqSendSome(conn); } + + /* + * PQfreeNotify - free's the memory associated with a PGnotify + * + * This function is here only for binary backward compatibility. + * New code should use PQfreemem(). A macro will automatically map + * calls to PQfreemem. It should be removed in the future. bjm 2003-03-24 + */ + + #undef PQfreeNotify + void PQfreeNotify(PGnotify *notify); + + void + PQfreeNotify(PGnotify *notify) + { + PQfreemem(notify); + } + + Index: src/interfaces/libpq/libpq-fe.h =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/libpq-fe.h,v retrieving revision 1.90 diff -c -c -r1.90 libpq-fe.h *** src/interfaces/libpq/libpq-fe.h 22 Mar 2003 03:29:06 -0000 1.90 --- src/interfaces/libpq/libpq-fe.h 25 Mar 2003 02:40:16 -0000 *************** *** 260,266 **** /* Simple synchronous query */ extern PGresult *PQexec(PGconn *conn, const char *query); extern PGnotify *PQnotifies(PGconn *conn); ! extern void PQfreeNotify(PGnotify *notify); /* Interface for multiple-result or asynchronous queries */ extern int PQsendQuery(PGconn *conn, const char *query); --- 260,267 ---- /* Simple synchronous query */ extern PGresult *PQexec(PGconn *conn, const char *query); extern PGnotify *PQnotifies(PGconn *conn); ! /* Exists for backward compatibility. bjm 2003-03-24 */ ! #define PQfreeNotify(ptr) PQfreemem(ptr) /* Interface for multiple-result or asynchronous queries */ extern int PQsendQuery(PGconn *conn, const char *query); Index: src/interfaces/python/pgmodule.c =================================================================== RCS file: /cvsroot/pgsql-server/src/interfaces/python/pgmodule.c,v retrieving revision 1.44 diff -c -c -r1.44 pgmodule.c *** src/interfaces/python/pgmodule.c 10 Mar 2003 22:28:22 -0000 1.44 --- src/interfaces/python/pgmodule.c 25 Mar 2003 02:40:18 -0000 *************** *** 2066,2072 **** } PyTuple_SET_ITEM(notify_result, 1, temp); ! free(notify); } else { --- 2066,2072 ---- } PyTuple_SET_ITEM(notify_result, 1, temp); ! PQfreemem(notify); } else { Index: src/test/examples/testlibpq2.c =================================================================== RCS file: /cvsroot/pgsql-server/src/test/examples/testlibpq2.c,v retrieving revision 1.7 diff -c -c -r1.7 testlibpq2.c *** src/test/examples/testlibpq2.c 7 Feb 1999 22:08:54 -0000 1.7 --- src/test/examples/testlibpq2.c 25 Mar 2003 02:40:18 -0000 *************** *** 98,104 **** fprintf(stderr, "ASYNC NOTIFY of '%s' from backend pid '%d' received\n", notify->relname, notify->be_pid); ! free(notify); break; } PQclear(res); --- 98,104 ---- fprintf(stderr, "ASYNC NOTIFY of '%s' from backend pid '%d' received\n", notify->relname, notify->be_pid); ! PQfreemem(notify); break; } PQclear(res);