From: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Cc: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se> |
Subject: | Re: Avoid use deprecated Windows Memory API |
Date: | 2022-09-15 12:58:40 |
Message-ID: | CAJ7c6TPzD4VsmpeYVT=kOyimMA0u59BztmhOunWxMcrjgsn1jg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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. 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.
> 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.
[1]: https://docs.microsoft.com/en-us/windows/win32/api/securitybaseapi/nf-securitybaseapi-initializeacl
[2]: https://docs.microsoft.com/en-us/windows/win32/api/securitybaseapi/nf-securitybaseapi-gettokeninformation
--
Best regards,
Aleksander Alekseev
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Replace-LocalAlloc-LocalFree-with-HeapAlloc-HeapF.patch | application/octet-stream | 4.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ranier Vilela | 2022-09-15 13:05:19 | Re: Avoid use deprecated Windows Memory API |
Previous Message | houzj.fnst@fujitsu.com | 2022-09-15 12:57:07 | RE: why can't a table be part of the same publication as its schema |