On 2013-10-28 16:02:36 -0400, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > It'd be relatively easy to add support for make check (not installcheck)
> > wrapping postgres in valgrind via pg_regress, but I am not sure that's
> > the best way to go.
>
> > I think defining an additional CFLAG (USE_VALGRIND) shouldn't be a
> > problem?
>
> CFLAGS doesn't seem to have anything to do with this. I'd be more
> inclined to add a "--wrapper=prog" argument to pg_regress and invoke
> it with something like --wrapper="valgrind --trace-children=yes".
Err. I am *obviously* not saying that it makes the backend run under
valgrind. But if we're going to have a buildfarm animal running
valgrind, it'd be useful to run to let it catch all errors it can
instead of hiding many of them via mcxt/aset.c? Right?
> The larger problem though is what you'd do with the output. There's
> enough false-positive noise from valgrind that I can't see having
> the buildfarm run just fail if there are any messages. What to do
> instead isn't very clear.
The false positives should be gone using the suppressions file we ship
these days (--suppressions=/path/to/pg/src/tools/valgrind.supp). We
might miss some more cases there, but it should be fairly easy to extend
it.
> It seems to me the most reasonable fix for this is to make
> TupleDescInitEntry notice that the passed "attributeName" points
> at the tupdesc's name field and not call namestrcpy if so.
> This would go with an API clarification stating that callers can
> pass that if they want the name field to be unchanged.
+1
> Another possibly-useful approach would be to hack namestrcpy itself
> to handle name == str specially. However, that's legitimizing a
> usage that's really a type cheat, so I don't like it as much, even
> though it might fix more cases besides this one.
-1
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
