Re: Avoid use deprecated Windows Memory API - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Avoid use deprecated Windows Memory API
Date
Msg-id CAEudQAp57HtShUY3wiw-k3DROx9Ncm4JbbHJAUEpWQydAQB1+w@mail.gmail.com
Whole thread Raw
In response to Re: Avoid use deprecated Windows Memory API  (Aleksander Alekseev <aleksander@timescale.com>)
List pgsql-hackers
Em qui., 15 de set. de 2022 às 09:58, Aleksander Alekseev <aleksander@timescale.com> escreveu:
Hi Ranier,

> use HeapAlloc instead, once LocalAlloc is an overhead wrapper to HeapAlloc.

Thanks for the patch.

Although MSDN doesn't explicitly say that LocalAlloc is _depricated_
+1 for replacing it.
The MSDN says:
" New applications should use the heap functions".
IMO Postgres 16, fits here.

I agree with Alvaro that it is unlikely to be
ever removed, but this is a trivial change, so let's keep the code a
bit cleaner. Also I agree that no benchmarking is required for this
patch since the code is not performance-sensitive.
In no time, the patch claims performance.
So much so that the open CF is in refactoring.


> These calls are not equal, the LocalAlloc calls zeroes out the allocated memory

I took a look. The memory is initialized below by InitializeAcl() /
GetTokenInformation() [1][2] so it seems we are fine here.

The patch didn't have a proper commit message and had some issues with
the formating. I fixed this. The new code checks the return value of
GetProcessHeap() which is unlikely to fail, so I figured unlikely() is
appropriate:

+       hDefaultProcessHeap = GetProcessHeap();
+       if (unlikely(hDefaultProcessHeap == NULL))

The corrected patch is attached.
Thanks for the review and fixes.

regards,
Ranier Vilela

pgsql-hackers by date:

Previous
From: Aleksander Alekseev
Date:
Subject: Re: Avoid use deprecated Windows Memory API
Next
From: Alvaro Herrera
Date:
Subject: Re: pgsql: Doc: Explain about Column List feature.