Excerpts from Kyotaro HORIGUCHI's message of jue jun 21 07:22:43 -0400 2012: > Hello, > > > > Seems like we missed the fact that we still did SvUTF8_on() in sv2cstr > > > and SvPVUTF8() when turning a perl string into a cstring. > > > > Right. > > I spent a bit longer time catching on pl/perl and now understand > what is the problem...
Hi, Thanks for the review. > > So I played a bit with this patch, and touched it a bit mainly just to > > add some more comments; and while at it I noticed that some of the > > functions in Util.xs might leak some memory, so I made an attempt to > > plug them, as in the attached patch (which supersedes yours). > > Ok, Is it ok to look into the newer patch including fix of leaks > at first? Yeah, I'm going to have to backpatch the whole of it so having full review is good. Thanks. > -- The others > > Variable naming in util_quote_*() seems a bit confusing, > > > text *arg = sv2text(sv); > > text *ret = DatumGetTextP(..., PointerGetDatum(arg))); > > char *str; > > pfree(arg); > > str = text_to_cstring(ret); > > RETVAL = cstr2sv(str); > > pfree(str); > > Renaming ret to quoted and str to ret as the patch attached might > make it easily readable. I think I'm going to refrain from this because it will be more painful to backpatch. > > Now, with my version of the patch applied and using a SQL_ASCII database > > to test the problem in the original report, I notice that we now have a > > regression failure: > snip. > > I'm not really sure what to do here -- maybe have a second expected file > > for that test is a good enough answer? Or should I just take the test > > out? Opinions please. > > > The attached ugly patch does it. We seem should put NO_LOCALE=1 > on the 'make check' command line for the encodings not compatible > with the environmental locale, although it looks work. The idea of separating the test into its own file has its merit; but instead of having two different tests, I'm going to have a single test and two expected files. That seems simpler than messing around in the makefile. -- Álvaro Herrera <alvhe...@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers