Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > >> Actually, my recommendation is to remove it altogether. The mechanisms > >> are in place to close allocated files after elog(), so why waste thought > >> and code space to release them manually? > > > Fix applied. There is a FileFree() just below this in the code: > > if (!pipe) > > FreeFile(fp); > > We don't need the if (!pipe) because this code is in an else of
^^^^ I should have said "in the _new_ code" above. > > if(pipe). For clarity, it seems the FreeFile call makes sense. > > Huh? That FreeFile is *necessary* because it is not in an elog(ERROR) > path; and by my reading the "if (!pipe)" is needed too. Woh, I know we need to keep this code: if (!pipe) FreeFile(fp); I was saying we don't need if (!pipe) in the new FreeFile() we just added. > We do have a fair amount of other code that releases resources just > before doing elog(ERROR); so Brent was just emulating code he saw > elsewhere... but it's a coding habit I think we should move away from. > If the resource in question would be released anyway during error > recovery, then I don't see the value of doing it "by hand"; it just > bloats the backend (and adds potential for error, as in this case). > The only exception I'd make is for the case of releasing a spinlock or > LWLock; it's better to release the lock ASAP so as not to block other > backends longer than necessary. Agreed. However, I have never seen it stated that file descriptors are freeded on elog(ERROR). I certainly didn't know that. If we are going to remove them, let's do it systematically. Let's state in the developers FAQ what elog(ERROR) frees automatically (file descriptors, memory in most contexts, anything else?) and then I can check all the elog(ERROR) cases and make sure this is done consistently. -- Bruce Momjian | http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 ---------------------------(end of broadcast)--------------------------- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly