possible repalloc() in icu_convert_case()

Started by Anton Voloshinabout 5 years ago3 messageshackers
Jump to latest
#1Anton Voloshin
a.voloshin@postgrespro.ru

Hello,

in src/backend/utils/adt/formatting.c, in icu_convert_case() I see:
if (status == U_BUFFER_OVERFLOW_ERROR)
{
/* try again with adjusted length */
pfree(*buff_dest);
*buff_dest = palloc(len_dest * sizeof(**buff_dest));
...

Is there any reason why this should not be repalloc()?

In case it should be, I've attached a corresponding patch.

--
Anton Voloshin
Postgres Professional: https://www.postgrespro.com
Russian Postgres Company

Attachments:

repalloc-in-adt-formatting.patchtext/plain; charset=UTF-8; name=repalloc-in-adt-formatting.patchDownload+1-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Anton Voloshin (#1)
Re: possible repalloc() in icu_convert_case()

Anton Voloshin <a.voloshin@postgrespro.ru> writes:

in src/backend/utils/adt/formatting.c, in icu_convert_case() I see:
if (status == U_BUFFER_OVERFLOW_ERROR)
{
/* try again with adjusted length */
pfree(*buff_dest);
*buff_dest = palloc(len_dest * sizeof(**buff_dest));
...

Is there any reason why this should not be repalloc()?

repalloc is likely to be more expensive, since it implies copying
data which isn't helpful here. I think this code is fine as-is.

regards, tom lane

#3Anton Voloshin
a.voloshin@postgrespro.ru
In reply to: Tom Lane (#2)
Re: possible repalloc() in icu_convert_case()

On 04.04.2021 19:20, Tom Lane wrote:

repalloc is likely to be more expensive, since it implies copying
data which isn't helpful here. I think this code is fine as-is.

Oh, you are right, thanks. I did not think properly about copying in
repalloc.

--
Anton Voloshin
Postgres Professional: https://www.postgrespro.com
Russian Postgres Company