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...


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

Á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:

Reply via email to