Add assertion for failed alloc to palloc0() and palloc_extended()
Hi,
I noticed that we have Assert(ret != NULL) in palloc() but not in
palloc0() so for consistency I decided to add it. I also added an
assertion that the MCXT_ALLOC_NO_OOM flag is set if alloc() returns
NULL to palloc_extended().
I feel that this might be useful since while palloc() is much more
common the OOM which causes alloc() to incorrectly return NULL could in
theory happen in any of the three functions.
Andreas
Attachments:
0001-Add-assertion-for-failed-alloc-to-palloc0-and-palloc.patchtext/x-patch; charset=UTF-8; name=0001-Add-assertion-for-failed-alloc-to-palloc0-and-palloc.patchDownload+3-2
On Sat, Mar 01, 2025 at 01:05:43AM +0100, Andreas Karlsson wrote:
I noticed that we have Assert(ret != NULL) in palloc() but not in palloc0()
so for consistency I decided to add it. I also added an assertion that the
MCXT_ALLOC_NO_OOM flag is set if alloc() returns
NULL to palloc_extended().I feel that this might be useful since while palloc() is much more common
the OOM which causes alloc() to incorrectly return NULL could in theory
happen in any of the three functions.
Hmm. Good points. All the MemoryContextMethods rely on
MemoryContextAllocationFailure() to handle the case of
MCXT_ALLOC_NO_OOM on failure (except alignedalloc which has no alloc
routine). Your two suggestions, one in palloc0() for the non-NULL
check, and the second one in palloc_extended() to make sure that we
have MCXT_ALLOC_NO_OOM set when the result is NULL, could catch
inconsistencies when implementing a new method.
In short, LGTM. Will apply if there are no objections.
--
Michael
On Mon, Mar 03, 2025 at 01:13:05PM +0900, Michael Paquier wrote:
In short, LGTM. Will apply if there are no objections.
And applied as 40d3f8274499.
--
Michael