Hi,
On 2019-11-19 10:16:02 -0500, Tom Lane wrote:
> vignesh C <vignesh21@gmail.com> writes:
> > createPQExpBuffer allocates memory and returns the pointer, there is a
> > possibility that createPQExpBuffer can return NULL pointer in case of
> > malloc failiure, but initdb's main function does not check this
> > condition. During malloc failure when pointer is accessed it results
> > in segmentation fault. Made changes to check and exit if
> > createPQExpBuffer return's NULL pointer. Patch for the same is
> > attached.
>
> I can't get excited about this, for several reasons.
>
> 1) The probability of it happening in the field is not
> distinguishable from zero, surely. I imagine you forced this
> failure by making a debugging malloc fail occasionally.
Agreed wrt this specific failure scenario. It does however seem not
great that callsites for PQExpBuffer ought to check every call for
allocation failures, in the general case.
I do think it'd be reasonable to move the cases where "graceful" dealing
with OOM isn't necessary ought to really use an interface that
internally errors out on memory allocation failures. Kinda thinking we
ought to slowly move such paths towards stringinfo...
> 2) If we really are out of memory at this point, we'd have just as good
> odds that some allocation request inside pg_log_error() would fail.
> There's no practical way to ensure that that code path remains free
> of malloc attempts. (Not to mention cleanup_directories_atexit().)
I wonder if, for frontend paths, a simplified error handling path would
be worthwhile for OOM paths. Doing only a write() or such to print an
error message.
Greetings,
Andres Freund
