Re: memory-related bugs

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: memory-related bugs
Date: 2011-09-06 19:00:42
Message-ID: 18540.1315335642@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ Sorry for letting this slip through the cracks ... I think I got
distracted by collation bugs :-( ]

Noah Misch <noah(at)leadboat(dot)com> writes:
> On Sat, Mar 12, 2011 at 12:44:29PM -0500, Tom Lane wrote:
>> Noah Misch <noah(at)leadboat(dot)com> writes:
>>> A suitably-instrumented run of "make installcheck-world" under valgrind turned
>>> up a handful of memory-related bugs:

>> Hmm, interesting work, but I don't think I believe in the necessity for
>> this kluge:
>>
> + else if (attributeName != &(att->attname))
> + namestrcpy(&(att->attname), attributeName);

I'm still of the opinion that there's no real need to avoid memcpy with
identical source and destination, so I didn't apply this first patch.

>> I wonder whether we should instead fix this by copying the correct tuple
>> length.

> Seems like a step in the wrong direction. We only use typlen and typbyval
> beyond the immediate context.

Well, the whole point of exposing the pg_type tuple is to avoid making
assumptions about what parts of it the type-specific analyze routine
will wish to look at. If anything we ought to move in the direction of
allowing the non-fixed fields to be accessible too. I didn't do that,
since it would imply an ABI change (to add a HeapTuple field to
VacAttrStats) and would therefore not be back-patchable. But I did
change the code to use SearchSysCacheCopy, which fixes this bug and
is readily extensible if we do decide to add such a field later.

I applied your third patch (for SJIS2004 conversion) as-is.

Thanks for the report and testing!

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2011-09-06 19:14:53 Re: timezone GUC
Previous Message Josh Berkus 2011-09-06 18:41:01 Re: Alpha 1 for 9.2