[HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Martijn van Oosterhout
Coverity picked up an error in dumpStdStrings() since last night. At line 1448 there's PQclear(res) yet it's used several times further down (lines 1452, 1454 and 1456). I'd actually suggest zeroing out res-tuples in PQclear so this sort of problem becomes much more obvious. Coverity bug 304 for

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Alvaro Herrera
Martijn van Oosterhout wrote: Coverity picked up an error in dumpStdStrings() since last night. At line 1448 there's PQclear(res) yet it's used several times further down (lines 1452, 1454 and 1456). I'd actually suggest zeroing out res-tuples in PQclear so this sort of problem becomes much

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes: Martijn van Oosterhout wrote: I'd actually suggest zeroing out res-tuples in PQclear so this sort of problem becomes much more obvious. Is it worthwhile to zero out the res-block array as well? Your patch isn't doing that, merely zeroing a local

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Martijn van Oosterhout
On Sun, May 28, 2006 at 12:00:33PM -0400, Tom Lane wrote: Another possibility is to just MemSet the whole PGresult struct to zeroes before free'ing it. Compared to the cost of obtaining a query result from the backend, this probably doesn't cost enough to be worth worrying about, and it would

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Tom Lane
Martijn van Oosterhout kleptog@svana.org writes: On Sun, May 28, 2006 at 12:00:33PM -0400, Tom Lane wrote: Another possibility is to just MemSet the whole PGresult struct to zeroes before free'ing it. Probably better actually, since by setting ntups to zero also, PQgetvalue will return a

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Alvaro Herrera
Tom Lane wrote: Martijn van Oosterhout kleptog@svana.org writes: On Sun, May 28, 2006 at 12:00:33PM -0400, Tom Lane wrote: Another possibility is to just MemSet the whole PGresult struct to zeroes before free'ing it. Probably better actually, since by setting ntups to zero also,

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Bruce Momjian
Alvaro Herrera wrote: Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Tom Lane wrote: Hm. But I think we'd *like* it to segfault; the idea is to make the user's programming error as obvious as possible. Is it worth the trouble to just zero out the pointer members of the

Re: [HACKERS] Error in recent pg_dump change (coverity)

2006-05-28 Thread Alvaro Herrera
Alvaro Herrera wrote: Done. They were actually four, not five. The one I mistakingly though was one was the notice processor hooks. The case Martijn was saying would be warned about by the memset approach, setting ntuples to 0, would actually be handled as a segfault, because functions