Noah Misch <noah@leadboat.com> writes:
> On Sun, Nov 17, 2024 at 01:26:38AM -0500, Tom Lane wrote:
>> arcarray = (struct arc **) MALLOC(totalinarcs * sizeof(struct arc *));
>> if (arcarray == NULL)
>> On a machine where malloc(0) returns NULL, this mistakenly
>> thinks that's an error.
> Either of those sound reasonable. The consequence of missing this hazard, a
> deterministic ERROR, is modest. This affects just one platform, in the oldest
> branches. There's a lack of complaints. To me, all that would make the
> one-line diff tempting.
I dug through the MALLOC and REALLOC calls in backend/regex/ and
convinced myself that this is the only one that's at risk. (Some
of those conclusions depend on the assumption that a regex NFA
never has nstates == 0, but I think that's okay: we create start
and end states to begin with and never remove them.) So the
one-liner fix is looking attractive. I'd prefer a malloc wrapper
for future-proofing if this code were likely to receive a lot of
churn in the pre-v16 branches, but that seems pretty improbable
at this point.
>> So the right answer seems to be to figure out why we didn't
>> back-patch that change.
> I don't recall a specific reason or see one in the discussion of commit
> bea3d7e38. It was done mainly to unblock commit db4f21e, which in turn
> unblocked commit 0da096d. The last commit is heavy, so I can understand it
> skipping the back branches. If I were making a (weak) argument against
> back-patching bea3d7e38, I might cite the extra memory use from
> RegexpCacheMemoryContext and children.
I think just on minimum-risk grounds, I wouldn't consider
back-patching bea3d7e38. I had more in mind a bespoke
three-line malloc wrapper function. But the one-line fix
seems sufficient for the problem at hand.
regards, tom lane
