Re: Missing checks when malloc returns NULL...

On Mon, Aug 29, 2016 at 11:13 PM, Aleksander Alekseev
<a(dot)alekseev(at)postgrespro(dot)ru> wrote:
>> Hello, Michael
>>
>> > I don't know how you did it, but this email has visibly broken the
>> > original thread. Did you change the topic name?
>>
>> I'm very sorry for this. I had no local copy of this thread. So I wrote a
>> new email with the same subject hoping it will be OK. Apparently right
>> In-Reply-To header is also required.
>>
>> > if (prodesc->user_proname == NULL || prodesc->internal_proname == NULL)
>> > + {
>> > + free(prodesc);
>>
>> I think that prodesc->user_proname and prodesc->internal_proname should
>> also be freed if they are not NULL's.
>>
>> > By the way maybe someone knows other procedures besides malloc, realloc
>> > and strdup that require special attention?
>>
>> I recalled that there is also calloc(). I've found four places that use
>> calloc() and look suspicious to me (see attachment). What do you think -
>> are these bugs or not?

./src/backend/storage/buffer/localbuf.c: LocalBufferBlockPointers =
(Block *) calloc(nbufs, sizeof(Block));
./src/interfaces/libpq/fe-print.c- fprintf(stderr,
libpq_gettext("out of memory\n"));
Here it does not matter the process is taken down with FATAL or
abort() immediately.

./src/backend/bootstrap/bootstrap.c: app = Typ =
ALLOC(struct typmap *, i + 1);
But here it does actually matter.

> I've just realized that there is also malloc-compatible ShmemAlloc().
> Apparently it's return value sometimes is not properly checked too. See
> attachment.

./src/backend/storage/lmgr/proc.c: pgxacts = (PGXACT *)
ShmemAlloc(TotalProcs * sizeof(PGXACT));
./src/backend/storage/lmgr/proc.c: ProcStructLock = (slock_t *)
ShmemAlloc(sizeof(slock_t));
./src/backend/storage/lmgr/lwlock.c: ptr = (char *)
ShmemAlloc(spaceLocks);
./src/backend/storage/ipc/shmem.c: ShmemAlloc(sizeof(*ShmemVariableCache));
./src/backend/access/transam/slru.c: shared->buffer_locks =
(LWLockPadded *) ShmemAlloc(sizeof(LWLockPadded) * nslots);
./src/backend/postmaster/postmaster.c: ShmemBackendArray = (Backend
*) ShmemAlloc(size);

The funny part here is that ProcGlobal->allProcs is actually handled,
but not the two others. Well yes, you are right, we really need to
fail on FATAL for all of them if ShmemAlloc returns NULL as they
involve the shmem initialization at postmaster startup.
--
Michael