Re: Avoid use deprecated Windows Memory API

Em qui., 15 de set. de 2022 às 09:58, Aleksander Alekseev <
aleksander(at)timescale(dot)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
<https://docs.microsoft.com/en-us/windows/desktop/Memory/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