possible repalloc() in icu_convert_case()

Started by Anton Voloshinover 5 years ago3 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

never appliedCI history
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